Skip to content

Use tokenizer to parse label quotes into tokens #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Jul 22, 2020
Merged

Use tokenizer to parse label quotes into tokens #170

merged 6 commits into from
Jul 22, 2020

Conversation

@lucaslorentz
Copy link
Owner

@lucaslorentz lucaslorentz commented Jul 22, 2020 •
edited
Loading

Improvements for #168

@lucaslorentz lucaslorentz changed the title Use tokenizer to parse label quotes in tokens Use tokenizer to parse label quotes into tokens Jul 22, 2020
francislavoie
francislavoie reviewed Jul 22, 2020
View reviewed changes
@francislavoie francislavoie mentioned this pull request Jul 22, 2020
Closed
@francislavoie
Copy link
Collaborator

How will it work for yaml if you need multiple args with or without quotes? Should backticks be used in yaml in general do you think?

lucaslorentz
lucaslorentz commented Jul 22, 2020
View reviewed changes
plugin/caddyfile/marshal.go
}

if strings.ContainsAny(key, ` "'`) {
if strings.ContainsAny(key, "\n\"") {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@francislavoie
Updated this logic, to make use of backticks whenever it can to improve readability

lucaslorentz
lucaslorentz commented Jul 22, 2020
View reviewed changes
plugin/caddyfile/testdata/labels/quotes.txt
without_spaces abcd
multiple "a b c d" abcd
back `{"some":"json"}`
escaped `a"b`
Copy link
Owner Author

@lucaslorentz lucaslorentz Jul 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@francislavoie
This is now a case, where we would not preserve the original quotes used to declare the token.
But the generator knows backticks generate a more readable string because it doesn't require double quote scaping.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@francislavoie
What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm indifferent? Feels like an edgecase that won't come up so often. 🤷‍♂️

@lucaslorentz
Copy link
Owner Author

lucaslorentz commented Jul 22, 2020 •
edited
Loading

I think you can chose between double-quotes or backticks.
All those options should work now:

mylabel: "\"arg 1\" \"arg 2\""
mylabel: "`arg 1` `arg 2`"
mylabel: '"arg 1" "arg 2"'
mylabel: "`arg\n1` `arg 2`"
mylabel: arg1 arg2

I'm not sure about these ones:

mylabel: |
  "arg 1" "arg 2"
mylabel: |
  `line1
  line2` otherarg

@francislavoie
Copy link
Collaborator

francislavoie commented Jul 22, 2020 •
edited
Loading

Ah right, single quotes helps the situation 👍

Is mylabel: arg1 "arg2" valid yaml?

@lucaslorentz
Copy link
Owner Author

lucaslorentz commented Jul 22, 2020 •
edited
Loading

Wow, according to https://yaml-online-parser.appspot.com/ that works very well. I'm surprised!

But this isn't:

mylabel: "arg1" "arg2"

@lucaslorentz
Copy link
Owner Author

@francislavoie
In caddyfile, can backtick be used to declare multiline strings? Like:

respond / `
<html>
....
</html>
`

@francislavoie
Copy link
Collaborator

francislavoie commented Jul 22, 2020 •
edited
Loading

Yeah, multiline backticks works:

{
	"apps": {
		"http": {
			"servers": {
				"srv0": {
					"listen": [
						":80"
					],
					"routes": [
						{
							"match": [
								{
									"path": [
										"/"
									]
								}
							],
							"handle": [
								{
									"body": "\n\u003chtml\u003e\n....\n\u003c/html\u003e\n",
									"handler": "static_response"
								}
							]
						}
					]
				}
			}
		}
	}
}

You can check easily with caddy adapt --pretty

The \u unicode sequences comes from golang's JSON marshaler when spitting the config structs back to JSON

@lucaslorentz lucaslorentz force-pushed the parse-label-quotes branch 2 times, most recently from 3d15482 to 02ac7f1 Compare July 22, 2020 21:03
@lucaslorentz lucaslorentz merged commit c3de60c into master Jul 22, 2020
@lucaslorentz lucaslorentz deleted the parse-label-quotes branch July 22, 2020 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@francislavoie francislavoie francislavoie left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lucaslorentz @francislavoie