search

rouge-ruby / rouge

A pure Ruby code highlighter that is compatible with Pygments
https://rouge.jneen.net/
Other
3.33k stars 735 forks source link

Extract regex to constant in HTML formatter #1904

Closed tancnle closed 1 year ago

tancnle commented 1 year ago

This helps to reduce multiple instantiations of escape regex variables in the HTML formatter. Also, add some stylish changes around the indentation.

This PR extracts partial changes proposed in https://github.com/rouge-ruby/rouge/pull/1670 by @robotmay.

I have verified that the change does not affect the HTML output via the visual tester.

Screenshot 2022-12-14 at 10 20 46 am

tancnle commented 1 year ago

@jneen Any thoughts on this PR since you have reviewed the related one? 🙏🏼

jneen commented 1 year ago

Do we have benchmarks for this? I'm a bit wary of perf related changes without actual benchmarks to back it up, since we've been sent on goose chases before.

tancnle commented 1 year ago

Thanks, @jneen.

Do we have benchmarks for this? I'm a bit wary of perf related changes without actual benchmarks to back it up, since we've been sent on goose chases before.

I don't think this is a perf-related change :) This PR does not include the gsub! implementation from the original PR.

jneen commented 1 year ago

Okay! It's fine by me then.

jneen commented 1 year ago

I should also say, gsub! should be safe, since the strings here come from regexp capture groups, which as far as I'm aware are created on match.

jneen commented 1 year ago

(FWIW we already mutate these strings to de-duplicate adjacent tokens of the same type in Lexer#continue_lex)