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

Redesign regular expression handling #39

Merged
merged 6 commits into from Mar 29, 2014

Conversation

Projects
None yet
2 participants
@lunaryorn
Contributor

lunaryorn commented Mar 25, 2014

Puppet Mode now marks regular expressions as strings in syntax-propertize-function, and later applies the regular expression face in a similar way to how we handle variable expansion in strings already.

It's a lot more complicated than the old approach, but has two distinct advantages imho:

  1. forward-sexp and friends behave more reasonably now on regexp literals. They'll treat the entire literal as a single expression, so when point is at the beginning of a regexp literal, and you press C-M-f, Emacs will move over the entire regexp literal instead of stopping at any expression inside.
  2. It'll make our future SMIE grammar easier, since we can now just treat regexps as strings without having to explicitly tokenize them, which isn't that easy.

Additionally, Puppet Mode will now highlight regexp literals only when they in valid contexts, i.e. in node names, selectors, cases and after match operators, so an expression like x / 2 > y / 5 won't be highlighted as regular expression literal anymore.

@lunaryorn lunaryorn changed the title from This PR reworks the handling of regular expression literals to Redesign regular expression handling Mar 25, 2014

lunaryorn added some commits Mar 25, 2014

Treat regexp literals as strings in proper contexts
Mark regexp literals as strings by modifying the syntax table
properties, to make them play more nicely with sexp navigation.
Notably, forward-sexp will not move across the entire regexp literal.

Also limit the fontification and treatment of regexp literals to
contexts in which they are allowed, that is, in cases or selectors, node
names, and after match operators.
Put property on body of regexp literal
Font lock keywords would miss the property change otherwise
Use generic string class
Recommended by the Emacs Lisp reference

@lunaryorn lunaryorn self-assigned this Mar 25, 2014

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Mar 26, 2014

@bbatsov Would you mind a little review?

@bbatsov

This comment has been minimized.

Contributor

bbatsov commented Mar 26, 2014

Sorry, mate. It's crazy at work and I forgot to review the code. I'll get to that in a few hours.

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Mar 26, 2014

@bbatsov Take your time. Won't get to do any more on this before the next weekend anyway.

@@ -759,21 +766,26 @@ string."
"Match a valid escape sequence before LIMIT."
(puppet-match-property 'puppet-escape 'double-quoted limit))
(defun puppet-syntax-propertize-match (property)
"Propertize a match with PROPERTY.
(defun puppet-match-regexp-literal (limit)

This comment has been minimized.

@bbatsov

bbatsov Mar 28, 2014

Contributor

I wonder if we should mark such functions as "private" (puppet--). I like it when it's apparent what's considered an API and what is not.

This comment has been minimized.

@lunaryorn

lunaryorn Mar 28, 2014

Contributor

Do we even have any API? I mean, this is a mode, not a library. I'd say everything is just internal API here, and the API is just what's provided by Emacs to interact with the current major mode.

This comment has been minimized.

@bbatsov

bbatsov Mar 28, 2014

Contributor

Fair enough. I'm just sharing an observation - sooner or later someone starts building "mode extensions" and would often use anything available in the mode's source code. Obviously this is not a problem for us...

This comment has been minimized.

@lunaryorn

lunaryorn Mar 28, 2014

Contributor

@bbatsov Let's keep things as they are then. We can still discuss the state an API should people ever start building extensions for our mode.

@bbatsov

This comment has been minimized.

Contributor

bbatsov commented Mar 28, 2014

The changes look good to me. 👍

lunaryorn added a commit that referenced this pull request Mar 29, 2014

Merge pull request #39 from lunaryorn/rework-regexp-highlighting
Redesign regular expression handling

@lunaryorn lunaryorn merged commit 948e492 into master Mar 29, 2014

1 check passed

default The Travis CI build passed
Details

@lunaryorn lunaryorn deleted the rework-regexp-highlighting branch Mar 29, 2014

lunaryorn added a commit that referenced this pull request Mar 29, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment