The great font lock fixup #12
Conversation
Use regexp-opt to generate the regex to match keywords instead of building it up by hand. The generated regex should be more efficient than just combining all the keywords with "\\|".
|
@bbatsov Btw, can we “unfork” somehow? Imho we won't ever contribute back to the old repo anyhow, so technically, we aren't a fork in the best Github sense anymore, and Github starts to get in our way. For instance, it wanted to open this PR against the old repo… Probably contact staff? |
|
Sure we can - just delete this repo and from GitHub and create it again from your local git repo. |
|
Won't that delete all issues? |
|
Unfortunately - yes. I didn't think about that. Maybe contacting the support is the best option at this point (or maybe the issues to be exported somehow?). As for the PR itself - some of the keywords are actually built-in functions that should be highlighted with Btw, we haven't worked on that much issues yet, so it might not be the worst idea. I've googled a bit and it seems some people share our pain and a clean repo import is only way to solve the problem... |
|
I'll contact support. If we don't get a response in a reasonable amount of time, we can still re-create the repository and migrate issues. It will be a lossy migration though, for while it is quite easy to get a complete list of issues from Github's API, it seems quite impossible to re-create them identically. For instance, you cannot for good reason create comments under another user's name via the API. Concerning the keywords, I have not yet taken a close look at the current regular expressions. I am not convinced though, that cherry picking the commit was a good idea actually. I have rather resolved my mind to more or less start from scratch with the keywords. It's a mess currently, as anything in this mode. I thought about writing a short Python script (or maybe refresh my little Ruby skills ;) ) to scrape Puppets documentation in order to obtain a complete list of reserved words and built-in functions. My intention is to write some ERT tests then to ensure that our font-lock keyword lists stays up to date, without us having to track Puppets documentation on each release. |
|
@bbatsov Github's support is truly awesome. This repo was detached not even ten minutes after writing to the support! |
|
@lunaryorn Awesome. I honestly did not expect this :-) |
|
Again on the subject of the PR - using a script to extract and verify keywords and built-in functions is a good idea. I personally prefer for us to do those in Ruby (for obvious reasons), but Python is an OK options as well. |
|
@bbatsov I'll try Ruby then, and leave my feeble attempt to your mercy on the review ;) |
Do you mean scraping the web site? The web documentation at docs.puppetlabs.com is mostly dynamically generated directly from the Puppet source (via the |
ghost
assigned 
|
@lunaryorn I'll merge this, since it's an improvement of the code, anyways (and it'll fix #16). |
I dedicate this PR to fixing the font locking.
Currently, it just contains the relevant commit from puppetlabs/puppet-syntax-emacs#4 cherry-picked, to give us a basic start, but there is muuuuuuuuuch to do still.
I ignored puppetlabs/puppet-syntax-emacs#8, for it's the wrong fix imho. The culprit is not ordering, it's the lack of symbol boundaries in the keyword regexp.
Essentially each single regexp should be reviewed, ported to
rx, usingsymbol-startandsymbol-endat the right places, and as muchsyntaxas possible.The string regexp is probably superfluous, since we having syntactic fontification. However, we should highlight variable references in strings, just like Ruby does. Let's have a look at ruby-mode for this one.
Gonna start tomorrow evening, in the mean time, feel free to comment, @bbatsov.