mcmaniac/highlighting-kate — comment on commit 828eaf6bcd66d60b69039a2ed70c59fbfbc2f2a5
File:  line 0
By jgm:

Thanks. These are sensible changes. I had a few suggestions for revision:

1. No version bump is needed, because 0.4 isn't released yet.

2. I'd be inclined to keep the error for "the impossible happened" in splitBySemi. This condition really is impossible, if I understand how 'break' works. So if it occurred, I'd want to be notified. Returning a Left value which is converted to a False by matchGlobs just loses the information.

3. I'd be inclined to have compileRegex and matchRegex return a KateParser Regex rather than an Either String Regex. That makes the code slightly cleaner -- the "fail" will just propagate out, and you don't need to complicate the code with tests for "Left" and "Right". Does that make sense?