magit-todos icon indicating copy to clipboard operation
magit-todos copied to clipboard

FIX: Inconsistent behavior between hl-todos and magit-todos

Open elfsternberg opened this issue 2 years ago • 3 comments

This patch addressses Magit-todos Issue 124.

Magit-todos has inconsistent behavior with hl-todos. If a keyword doesn't start with a whitespace character (using the elisp-regexp symbol blank), hl-todos will find it, but magit-todos will not.

This has consequences for documentation writers who typically leave their todo tasks in-line with the documentation, using the "to come" convention of [TK: Do something here]. Since the keyword "TK" does not have a leading space, hl-todos will find it, but magit-todos will not.

This patch replaces the requirement that the keyword be proceeded with a whitespace character with a more general case of (not alphanumeric). This creates behavior that matches hl-todos more closely. This change is only applied to the "non-org" branch of the macro.

elfsternberg avatar Mar 24 '22 19:03 elfsternberg

I'm not sure if this qualifies as a fix or a change. I don't think I necessarily intended that magit-todos behave exactly the same as hl-todo in this regard; the use of hl-todo in this package is mostly for convenience regarding display faces.

Anyway, if this is helpful, I don't mind making this change. But it should probably be done with a customizeable variable, similar to magit-todos-keyword-suffix. Regarding that, see also #69 and #118.

What do you think? Thanks.

alphapapa avatar Mar 26 '22 05:03 alphapapa

FWIW, I would appreciate the change even though I'm not using the documentation writers' conventions. As a case in point, i stumbled upon this issue because of comment lines like #TODO (without a space) or # pragmatic choice/TODO. Granted, these may not be following conventions and it might be argued in this specific example that # TODO: reconsider whether this is acceptable as a pragmatic choice would be preferable, but these things tend to come naturally when jotting down a quick aside while keeping focused on something else. And when in doubt, I'd prefer the TODOs section to err on the side of containing too much, particularly with capitalised keywords that aren't likely to occur by coincidence. Otherwise, I expect to keep searching for those keywords by other means because I could never be sure whether some of them escape me because of punctuation.

tlotze avatar Sep 02 '22 09:09 tlotze

See the second commit of PR #136: The same logic that fixes #135 can be applied to the beginning of the keyword, thereby solving the present issue in a way that doesn't conflict with a possible addition of a customizable prefix regex as discussed in this PR.

tlotze avatar Sep 03 '22 20:09 tlotze