Comprehensive regular expression support

[noprompt]

Whew! This was masochistic. 98% of the patterns should be covered here. (I skipped \Q..\E patterns because I wasn't sure the best way to handle them.) Overall, I think this is much more comprehensive than other language syntax files I've seen. It will be fun updating my color scheme to take advantage of them.

Most of the new syntax groups are linked to Special because that seemed to make the most sense.

Alright! I'm gonna take a little break from this now and check back later.

[noprompt]

Just a heads up, somehow my fork got a messed up and I'm not very good a fixing that sort of thing. How ever you decide to apply this patch is up to you. I'll have to fix my fork later.

[guns]

This is really awesome. No kidding about the masochism; this was pretty low on my TODO list for that reason :).

I merged your changes into a branch called noprompt-regexp:

https://github.com/guns/vim-clojure-static/tree/noprompt-regexp

I am currently working on a test file for the syntax now that it's grown reasonably complex. I also see at least one amendment (multiple adjacent flags are not matched: (?mix), and I am wondering if SpecialChar isn't more applicable than Special for the Regexp classes (the distinction I made between the two is that Special matches discrete tokens, while SpecialChar matches subtokens; e.g. Clojure special-forms link to Special, but reader metacharacters link to SpecialChar).

But anyway, this is stellar! Let's collaborate on the noprompt-regexp branch before we merge into master.

[noprompt]

All of this sounds great. Nice catch about the multiple flags. It shouldn't be too difficult to rework the expression to handle them. Also linking to SpecialChar instead of Special makes perfect sense to me and will probably end up being visually consistent with color schemes.

Thanks for creating the new branch and I appreciate your acceptance of these contributions. I've been coding a lot today, so I'll probably resume work on this tomorrow.

[noprompt]

Sorry for the slight lag on this. I've pushed the changes to the new branch. In addition to permitting multiple flags, I've added the optional - switch which was missing. So these will work all work correctly:

#"(?mix:X)"
#"(?-mix:X)"
#"(?mix-mix:X)"

As you've suggested I've switched most everything over to link to SpecialChar. clojureRegexpSpecialChar still links to Character. I think this makes sense because it provides an additional visual queue when working with them. If you disagree, I'm cool with linking it to SpecialChar. Since I maintain my own color scheme I can always tweak the highlighting to my liking. :)

Also, the clojureRegexpChar syntax group was dropped because it was incorrect and a subtle bug with matching control characters was patched. I'm excited about getting this merged!

[guns]

Outstanding. I think having a distinction between the character escapes and regexp specials is nice.

Please allow me to push a test suite (using clojure.test, shelling out to vim) before we merge to master; I think it will take me a few hours tonight. Then we can identify any other corner cases, and document which regexp specials are deliberately not matched.

[noprompt]

Looking forward to it!

[guns]

Okay, I've pushed up the beginnings of the syntax tests. You can start an nrepl from the clj/ subdirectory and eval the test from there, or do a lein test from clj/. The tests take a couple of seconds to run because they shell out to vim.

I added tests for numbers (which caught some new bugs!) and some initial tests for character escapes in regular expressions (BTW I renamed clojureRegexpSpecialChar to clojureRegexpEscape to mirror clojureStringEscape).

Some of the tests are currently failing, but I would like your feedback on which of these cases you are ignoring, which are difficult to get right, and which are actually errors. It would be great if you could start filling out the rest of the tests for java-regexp-literals-test. I think I've set a reasonable convention, but if there's anything you want to change, feel free to give it a try.

Thanks!

[noprompt]

This is pretty sweet. Creating this test suite is nothing short of brilliant. Right off the bat I was able to fix two problems (just pushed the fixes). Nice work!

... I would like your feedback on which of these cases you are ignoring, which are difficult to get right, and which are actually errors.

Ignoring

I'm ignoring \x{h...h} because, honestly, I'm not familiar with it and was confused by the definition. If you can give me an example, I can write a pattern for it.

Also, I've ignored comments which are permitted inside (?x) groups. I can study the ruby syntax file for ideas on how to implement this but I've skipped it for now.

Difficult

Previously I was ignoring \Q..\E patterns because I was a little unsure about the right approach. The solution is simple but it's slightly misleading:

syntax region clojureRegexpQuote start="\v\<@!\\Q" end="\\E"

It's misleading because, and I could be wrong, but it appear's if you drop the \E all characters until the end of the expression are quoted:

user=> (re-find #"\Qabc\E" "abc")
"abc"
user=> (re-find #"\Qabc" "abc")
"abc"

The documentation doesn't mention this behavior but I'm assuming from the results above my assumption is correct. Since I couldn't figure out a way to handle the case when the \E is dropped I decided to leave it out. On the other hand, implementing it as I've described is a nice way to use syntax highlighting to enforce good coding style.

Errors

Well a few of these are fixed already and it's just a matter of writing the remaining tests to check everything else. The convention in the test suite is clear, so I'm rolling with it. If I need to make any changes, I'll be sure to loop you in on them and get your thoughts.

Again, I can't tell you how impressed I am by the test suite. I've looked at several projects working on syntax files and have maybe seen a test suite once. Anyway, I'll be pushing more code tonight.

Edit: Update ignore section

[noprompt]

So I've gone ahead and written tests for all the current clojureRegexp* patterns (which was probably more exhausting than writing the patterns themselves!). The tests were a huge boon though and helped fix subtle bugs.

A few quick notes/questions:

• \., \+, \*, \[, \], \{, \}, \(, and \) have been added to clojureRegexpEscape (even though they're not documented)
• \Q..\E patterns have been added as clojureRegexpQuote but let's still consider the pro's and con's before deciding whether or not we want to include this
• Should we extract java.lang.Character classes and unicode classes in to their own syntax groups per the docs?
• Should we extract out ranges and negations [^ ...] ?
• I'm planning to add && once I figure out the best way to do it.

That's it for now. I've updated the branch and breaking on this for today! :)

[guns]

I couldn't post my comment through this text field for some reason, so
here is a gist:

https://gist.github.com/guns/5156348

[noprompt]

Thanks for solving the "\Q..\E" problem and explaining the solution. It helped a lot.

I think if we want to be complete, we can build a simple Unicode spec parser...

Completion is always a good thing IMHO and parsing the spec sounds fun. Is it overboard? Well, as long as it's not going to impact performance, and I doubt it will, it's probably fine. Just for kicks, I was able to use a little JavaScript from the browser console and lift the unicode categories from the Android docs with almost no effort.

$($('table')[1]).
  find('td:first-child').
  text().
  split(' ').
  filter(function(x) { 
    return x.length > 1 
  }).join("|")
// => "Cn|Cc|Cf|Co|Cs|Lu|Ll|Lt|Lm|Lo|Mn|Me|Mc|Nd|Nl|No|Pd|Ps|Pe|Pc|Pi|Pf|Po|Sm|Sc|Sk|So|Zs|Zl|Zp"

... and also use the JVM to find the java.lang.Character is* method names

I just took char of this one (couldn't resist the pun). It turns out not all java.lang.Character/is* patterns are valid but excluding the invalid ones was pretty trivial.

[noprompt]

Going back to the spec parser. We'll probably want to extract the (try ... (catch ...)) from java-char-class-names in to an is-char-class? function or something. That way when we actually parse out all the class names we can make sure they're compatible before dumping them. This shouldn't be hard to do.

Regarding the \x{h...h} patterns. They seem to cause trouble for me in.

Clojure 1.5.0
user=> (re-find #"\x{10300}" "bc")
PatternSyntaxException Illegal hexadecimal escape sequence near index 2
\x{10300}
  ^  java.util.regex.Pattern.error (Pattern.java:1713)

I don't feel pressured at all to work on these problems at all. Regular expressions, parsing, and text manipulation problems are my favorite. Without a doubt, once this is complete, this syntax file will have arguably some of the most robust and correct regular expression pattern highlighting out there. It's very encouraging.

[noprompt]

By the way I think the problem you were having posting that last comment had to do with that special A looking character. You'll notice it's missing in my previous comment. Might be worth mentioning to the Github staff.

[guns]

I think if we want to be complete, we can build a simple Unicode spec parser...

Completion is always a good thing IMHO and parsing the spec sounds fun. Is it overboard? Well, as long as it's not going to impact performance, and I doubt it will, it's probably fine.

Great, I'm glad you're game.

... and also use the JVM to find the java.lang.Character is* method names

I just took char of this one (couldn't resist the pun).

Oh you pronounce that kare? I think I must pronounce all my programming lingo wrong…

It turns out not all java.lang.Character/is* patterns are valid but excluding the invalid ones was pretty trivial.

Interesting. Thanks for implementing that. Could you please tweak java-char-class-names so that it contains the literal syntax match line, like syntax-keywords and completion-words?

Also, it looks like you used s/split instead of string/split on line 85 (the ns alias is string).

And could you please rebase your changes off the new merge I just pushed? We've gone slightly out of sync.

Going back to the spec parser. We'll probably want to extract the (try ... (catch ...)) from java-char-class-names in to an is-char-class? function or something. That way when we actually parse out all the class names we can make sure they're compatible before dumping them. This shouldn't be hard to do.

Yes, makes sense.

Regarding the \x{h...h} patterns. They seem to cause trouble for me in.

Clojure 1.5.0
user=> (re-find #"\x{10300}" "bc")
PatternSyntaxException Illegal hexadecimal escape sequence near index 2
\x{10300}
^ java.util.regex.Pattern.error (Pattern.java:1713)

Maybe this is platform dependent? Here is my java -version:

java version "1.7.0_17"
OpenJDK Runtime Environment (IcedTea7 2.3.8) (ArchLinux build 7.u17_2.3.8-1-x86_64)
OpenJDK 64-Bit Server VM (build 23.7-b01, mixed mode)

I don't feel pressured at all to work on these problems at all. Regular expressions, parsing, and text manipulation problems are my favorite.

Then that's the last time I'll mention it. :)

Without a doubt, once this is complete, this syntax file will have arguably some of the most robust and correct regular expression pattern highlighting out there. It's very encouraging.

Yes, totally agree. This is great stuff, and will trump ruby's implementation, which is one of the most feature-rich syntaxes I've seen. Java, lacking regexp literals, has none to begin with.

By the way I think the problem you were having posting that last comment had to do with that special A looking character. You'll notice it's missing in my previous comment. Might be worth mentioning to the Github staff.

The irony of talking about Unicode issues is that sometimes you cannot actually directly mention the issues at hand. I wonder if this is JavaScript's fault; I seem to remember some hubbub about nodejs and >BMP characters a few months back.

Did you have any thoughts about this:

In addition, in order to hilink only the delimiters \Q and \E, a subregion called clojureRegexpQuoted is created matching only the contents of the clojureRegexpQuote region. This subregion is linked to the normal clojureRegexp group, and the outer region remains linked to Character (although, do you think it may be better to link in the same style as clojureRegexpBoundary, since the \Q is a delimiter and not a character?)

[noprompt]

Oh you pronounce that kare?

Haha! Yeah. To me, pronouncing it "char" as in charbroiled seems a little strange. I always thought it made more sense to pronounce it "care" since it's short for character. That's how it works with int, right?

The problem with \x{...} on my machine is definitely a platform issue:

java version "1.6.0_37"
Java(TM) SE Runtime Environment (build 1.6.0_37-b06-434-11M3909)
Java HotSpot(TM) 64-Bit Server VM (build 20.12-b01-434, mixed mode)

Looks like I'm upgrading.

I've fixed the issue with s not being string. I had been experimenting in a separate buffer and must have been asleep at the wheel when copying it over. Shameful. The remaining issues with java-char-class-names will be fixed shortly.

Thoughts on \Q...\E? Linking the \Q and \E parts to clojureRegexpBoundary make sense. You're right. It's a delimiter. The subregion might be better off linked to Character or clojureRegexpEscape though. My concern with linking it to clojureRegexp is that it won't visually communicate the significance of \Q..\E to the programmer. But that could just be my graphic design sense poking through.

[guns]

Oh you pronounce that kare?

Haha! Yeah. To me, pronouncing it "char" as in charbroiled seems a little strange. I always thought it made more sense to pronounce it "care" since it's short for character.

Makes sense :) I guess I just enjoyed saying char *buf; as char-star-buf

That's how it works with int, right?

I'm going to start calling that "eint" to get on people's nerves.

The subregion might be better off linked to Character or clojureRegexpEscape though. My concern with linking it to clojureRegexp is that it won't visually communicate the significance of \Q..\E to the programmer.

What about linking it to String? It seems better than character or regexpEscape because those deal with individual characters, while \Q…\E marks off a whole string of chars.

[noprompt]

The idea to use String did cross my mind but I wasn't sure. Since you've also made this consideration, I think we should use it.

[guns]

Okay, then that's settled.

I agree with you now about naming the POSIX, Java, and Unicode \p{…} classes. I think you should go ahead and differentiate those, if only to signal to someone looking at the code. Plus, it may be possible to use the Unicode group for ClojureScript.

Here's another bit about the \p{…} classes according to http://www.regular-expressions.info/unicode.html:

(re-find #"\p{L}" "Matches any letter") ; "M"
(re-find #"\pL\pP" "Single letter classes can omit braces!") ; "s!"

This is quite a learning experience.

I'll refrain from pushing to noprompt-regexp and wait for you to push the next set of changes.

[noprompt]

Good deal. Breaking them up will also help a bit with code generation. It certainly has been a huge learning experience for me too! Thanks for doing all the research. 👍

[noprompt]

Whoa. Talk about crazy. Just pushed up the some work on the \p{..} style patterns. It turns out there were a few more gotchas! Not every bracket expression can have an Is part. For example \p{javaISOControl} will work but not \p{IsjavaISOControl}. On the other hand \p{javaDigit} and \p{IsjavaDigit} both work. Talk about throwing a wrench in the machine. And the same issue is true for the unicode expressions. It's not a big deal because it's easy to check and filter those out.

By they way, I updated Java so it should be printing out only what's currently valid.

java version "1.7.0_12-ea"
Java(TM) SE Runtime Environment (build 1.7.0_12-ea-b08)
Java HotSpot(TM) 64-Bit Server VM (build 24.0-b28, mixed mode)

My incremental version looks like it's less than yours hopefully that shouldn't mean anything significant. I'm taking a break and coming back to this later. I think all that's left here is the Posix expressions, which should be a push over.

Hopefully tomorrow I can start picking up the other small issues and write tests for them. Maybe we can ship this by the end of the weekend. 👍

[guns]

For example \p{javaISOControl} will work but not \p{IsjavaISOControl}

Unfortunately, both work on my machine. This may actually be undefined behaviour. I think we should only allow the \p{java…} construction without the leading Is since that seems to be the most consistent.

Also curious is that the Unicode block classes can occur with underscores, or without (but not some), and that not all the blocks in the current spec are supported:

http://www.unicode.org/Public/UNIDATA/Blocks.txt

Unicode actually has a section on implementing regular expression engines, but the actual Java implementation will obviously take precedence for us.

I plan on kicking back a few beers with the reference implementation of java.util.regex.Pattern and the relevant Unicode sections tonight to sort this all out. Then, if you can push the final POSIX changes, I can make some informed decisions about these Unicode and Java corner cases, and we can merge with master on Monday night.

Sound like a plan?

[noprompt]

Sounds like a plan to me. My allergies were kicking my ass all weekend so I haven't done any work since Friday. The valley this time of year can be rough.

Unfortunately, both work on my machine

It figures. I'm wondering if it has more to do with the OS than Java though (I'm running OS X). Spinning up a new Linux VM and checking is possible, but since you're volunteering to handle the edge cases I'll hold off on that for now.

I'm about to head out for dinner and, now that you mention it, a beer. When I get back I'll wrap up POSIX, the \pX expressions, and drop the Is part from the java generation.

Cheers! 🍻

[noprompt]

Unfortunately, both work on my machine

It turns out I actually didn't not have the latest JDK for OS X. My version now matches yours and, sure enough, this problem has gone away. I've decided to hold off on dropping Is from clojureRegexpJavaCharClass group until you've had a chance to determine whether it's undefined behavior. That way we don't drop anything too soon.

POSIX and \pX expressions should be good to go.

I couldn't resist parsing the spec and generating the expressions from them. The print out, as you can imagine, is pretty crazy. Part of me feels like going the extra mile and optimizing the generated expressions to limit the amount of backtracking - which is going to be through the roof for clojureRegexpUnicodeCharClass. I'm not sure what Vim does under the hood to optimize regular expressions but it might be worth it later on. It's also an interesting problem. The smaller expressions I'm doing by hand.

Other than that, I think what we've come up with is more than acceptable. We've covered 99% of what most programmers will likely ever use in a regular expression and the extra 1% is icing on the cake. I think the only thing else I could ask for is comment highlighting but I can work on those this week. This may not be possible though unless there's some clever way to know if the group contained the x modifier. Ruby has it easy with (?#).

Anyway, I'm looking forward to hearing about whatever you dig up from reference implementation!

🍰

Edit: Note about comments.

[guns]

Just want to let you know that I am still working on the Unicode cases. Everything looks great and hopefully I'll be able to merge and push tonight. I'd like to post to the VimClojure group for testers, but the true extent of the regexp matching will be invisible without a colorscheme that takes advantage of the new groups.

Do you have a colorscheme that I could promote to the list for people interested in these changes?

edit: s/syntax file/colorscheme/

[noprompt]

Awesome man! I'm so excited.

Do you have a colorscheme that I could promote to the list for people interested in these changes?

I never thought you'd ask! 😄 My personal theme lite-brite is always being updated. Now that we've settled on highlight names and I know this is going to be merged, I'll take a few minutes to add support for the new syntax groups.

[guns]

Now that we've settled on highlight names and I know this is going to be merged, I'll take a few minutes to add support for the new syntax groups.

Actually, there will be a couple of very minor changes (\p{} classes are referred to as property char classes) and additions (Unicode groups are split up into sections). Nothing major, I swear!

lite-brite looks great. I'll ping you after I push tonight.

[noprompt]

No worries. I can always readjust things once everything is finalized and merged.

[noprompt]

How's everything coming along? Anything else I can do?

[guns]

I'm sorry for the holdup; I've had some trouble finding time to hack this week. I finished the majority of the changes on Tuesday but I held off committing and pushing until I had a chance to review the fringes. Thanks for being patient!

Here are a summary of the changes in the new push. I'd like your input before I merge to master.

Character Property Classes

Unfortunately, the Unicode specs for character property names do not exactly match the reference implementation (as you know).

So now all of the Character Property Classes are now derived from reflection, and the majority from private fields. I know this is not acceptable in normal programs, but the data was just too good to pass up, especially since the private data included aliases that were not programmatically generated from the official names.

Property Prefixes

Is, In, and Isjava prefixes now follow the documentation instead of the implementation.

• POSIX properties have no prefixes and are matched case sensitively
• Java character properties follow the s/^is/java/ rule precisely and are matched case sensitively
• Unicode general category properties are optionally prefixed by Is, gc=, or general_category= and then are matched case sensitively
• Unicode script properties must be prefixed by Is, sc=, or script= and then are matched case INsensitively
• Unicode block properties must be prefixed by In, blk=, or block= and then are matched case INsensitively
• Unicode binary properties must be prefixed by Is and then are matched case INsensitively

Code structure

• Test file cleaned up and newly broken tests fixed
• The generate namespace was refactored to separate source data from vimscript literals

Syntax file

• \. removed from clojureRegexpEscape for reasons discussed earlier
• \x{…} range limited to 6 (hex value of Character.MAX_CODE_POINT is 0x10ffff)
• clojureRegexpBracketExp renamed to clojureRegexpCharPropertyClasses
• Dangling reference to clojureRegexpSpecialChar removed
• Linked clojureRegexpQuoted to clojureString
• Linked clojureRegexpQuote to clojureBoundary

(So actually, there were no visible syntax group changes)

---

I think that covers it! I haven't added new tests yet because I wanted to push sooner than later.

Sorry that your work on parsing the specs got replaced; I hope you'll agree that the wordlists procured from the JVM are different enough (neither subset nor superset) from the Unicode standards to merit just prying it out from Java's greedy private fingers.

[noprompt]

Everything looks awesome to me. I like the work you've done on refactoring/renaming and, honestly, I can't think of anything to pick a bone with. Nice job!

Sorry that your work on parsing the specs got replaced; I hope you'll agree that the wordlists procured from the JVM are different enough (neither subset nor superset) from the Unicode standards to merit just prying it out from Java's greedy private fingers.

Actually, the changes you are made fantastic. It makes more sense to lift what we need from the JVM and get it right rather than parsing the spec and only getting so close. Java isn't a language I know and reading through the new code has taught me a few things.

At any rate, I'm happy with all of this. If you're ready to merge, I have no objections.

Great job!

[guns]

Alright, then this wraps this up! Thank you very much for stepping up to the plate and getting this done. Would've taken me months to tackle it, if at all.

If lite-brite is ready, I'll post a message to the VimClojure group for testers and also start adding more tests.

[guns]

Okay, I posted to the list:

https://groups.google.com/forum/?fromgroups=#!topic/vimclojure/8V8NFa1qFu0

[noprompt]

Awesome! I'll be pushing some updates to the theme here in a moment to take advantage of the new groups. Sorry for the lag on that, I was in Yosemite all day!