Skip to content
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.

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

Extend valid set of quoted labels #65

Merged
merged 2 commits into from
Jan 12, 2018
Merged

Conversation

Gabriella439
Copy link
Contributor

Related to #64

This adds : to the list of supported characters for Dhall labels and
no longer treats the initial character of a quoted label differently

Related to #64

This adds `:` to the list of supported characters for Dhall labels and
no longer treats the initial character of a quoted label differently
@Profpatsch
Copy link
Member

Doesn’t this just move the problem? Now we have an arbitrary list of legal symbols, plus one of them is also used as the separator.

I’m not sure what the more general way what be. Escaping?

@Gabriella439
Copy link
Contributor Author

The reason I explicitly whitelist allowed characters instead of accepting a broader class of letters is due to the potential for identifier name obfuscation by using visually similar characters.

There actually is a Unicode standard with various alternatives for how to implement broader identifier names:

http://unicode.org/reports/tr31/

... but I decided to skip that for now since it increases the complexity of implementing DHall

@Profpatsch
Copy link
Member

Profpatsch commented Jan 8, 2018

Full ack, I’m always for explicit symbol sets. But does it make sense to expand those manually for some use cases? Except if adding : means all yaml symbols are valid now, then it’s probably a good idea.

Update: Just saw #64, which fixes that problem in a general way. So maybe it’s better to not introduce : in symbols? I can imagine that makes syntax highlighting for editors harder to implement (since editors mostly use regex for implementing simple syntax highlighting).

@Gabriella439
Copy link
Contributor Author

@Profpatsch: Note that this change is only for quoted labels (i.e. surrounded by backticks). That means that a colon is still not allowed in a normal unquoted label:

foo     -- Legal label
fo:o    -- Not a legal label; parsed as "fo : o"
`fo:o`  -- Legal label

This also doesn't prevent editors from using a regular expression for matching symbols. The regular expression is:

[A-Za-z0-9_][A-Za-z0-9_/-]*|`[A-Za-z0-9_/-:]+`

@Profpatsch
Copy link
Member

Ah nice, I wasn’t even aware that labels could be quoted with backticks.

@Gabriella439 Gabriella439 merged commit 7a8744b into master Jan 12, 2018
@Gabriella439 Gabriella439 deleted the gabriel/colon-labels branch January 12, 2018 17:26
Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this pull request Jan 18, 2018
Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this pull request Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants