Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Closes #200 Makes escaped hex include lowercase hex chars#201

Closed
ValarDragon wants to merge 2 commits intoatom:masterfrom
ValarDragon:master
Closed

Closes #200 Makes escaped hex include lowercase hex chars#201
ValarDragon wants to merge 2 commits intoatom:masterfrom
ValarDragon:master

Conversation

@ValarDragon
Copy link
Copy Markdown

Description of the Change

This makes the atom interpreter recognize lowercase hex characters in escaped hex sequences.

Alternate Designs

None were proposed, this is a simple bug fix. The original developer likely forgot that lowercase hex chars can be used.

Benefits

Escaped hex with a lowercase hex character such as "\x4a" will now be recongized as an escaped hex sequence.

Possible Drawbacks

None

Applicable Issues

#200

'13':
'name': 'constant.character.escape.vertical-tab.python'
'match': '(\\\\x[0-9A-F]{2})|(\\\\[0-7]{3})|(\\\\\\n)|(\\\\\\\\)|(\\\\\\")|(\\\\\')|(\\\\a)|(\\\\b)|(\\\\f)|(\\\\n)|(\\\\r)|(\\\\t)|(\\\\v)'
'match': '(\\\\x[0-9A-Fa-f]{2})|(\\\\[0-7]{3})|(\\\\\\n)|(\\\\\\\\)|(\\\\\\")|(\\\\\')|(\\\\a)|(\\\\b)|(\\\\f)|(\\\\n)|(\\\\r)|(\\\\t)|(\\\\v)'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The [0-9A-Fa-f] part can probably be replaced by \\h, which is the shortcut for hex in Oniguruma.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks! Fixed that in the second commit.

@winstliu
Copy link
Copy Markdown
Contributor

Thanks. One more thing - could you please add a few specs so that this behavior won't regress? I can help you out if you're having trouble how the specs work.

@ValarDragon
Copy link
Copy Markdown
Author

ValarDragon commented May 20, 2017

Could you please help me out, I don't actually know how the specs coffee file works.

@ValarDragon
Copy link
Copy Markdown
Author

Is there documentation about what should be put in the specs coffeescript file?

@winstliu
Copy link
Copy Markdown
Contributor

Hi @ValarDragon, sorry for the delay in getting back to you.

First of all, I'd like to apologize for my review comment, which I've recently found out is non-standard regex behavior (\h means different things using different regex engines). Could you please revert that back to [0-9A-Fa-f]? Sorry about that.

Regarding specs, here's a simple example that you can build off of: https://github.com/atom/language-javascript/blob/4f41fa5ee25a8059175cc073866d2a699dfbf280/spec/javascript-spec.coffee#L228-L233. You will need to change the scope names and alter the tested line to test the changes of this PR. If you still need help just ask. You can get scopes by using the editor:log-cursor-scope command.

@kbrose kbrose mentioned this pull request Oct 18, 2017
@winstliu winstliu closed this Oct 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants