ankurdave / color-identifiers-mode

Emacs minor mode to highlight each source code identifier uniquely based on its name
308 stars 23 forks source link

Defaulting to `cc-mode-get-declarations` instead of `scan-identifiers` #94

Closed Hi-Angel closed 1 year ago

Hi-Angel commented 1 year ago

This is mostly a refactoring task.

There are multiple bugs about wrong text being colorized, such as #40 or #62. Both #40 and #62 can be solved by making "scan-fn" color-identifiers:cc-mode-get-declarations (like in #93).

cc-mode-get-declarations and scan-identifiers are similar in what they do, but the main difference is that scan-identifiers is called against almost every word, whereas cc-mode-get-declarations is only called on words that have properties set. In terms of code the difference is that the former mostly uses re-search-forward to move through buffer, whereas the latter does that only with next-property-change. (yes, I know next-property-change is called in scan-identifiers too, but only if re-search-forward fails, which I don't think happens too often).

What I can gather from this difference is that these two functions have different usecases:

With that said, I don't think using scan-identifiers as the default scan-fn function is correct. More likely, the cc-mode-get-declarations should be renamed to sound more generic, and be the default scan-fn.

Hi-Angel commented 1 year ago

yes, I know next-property-change is called in scan-identifiers too, but only if re-search-forward fails, which I don't think happens too often

Oh, btw, it may not be obvious from the code, so to make it clear: when scan-identifiers checks a face at point and get nil because face isn't set, it checks if identifier-faces has nil. And it does! As result, face-property check succeeds very often, so the execution skips a call to next-property-change and goes further where a call to re-search-forward resides.