loosen tag restriction to allow all non-whitespace, after the first char

[smemsh]

This patch allows tags to be any non-whitespace characters, excepting the first char, which still must be an "identifier" character (see Lexer::isIdentifier()), in particular so that "task calc" works with negative numbers and subtraction. Apparently CLI2 doesn't know the command yet when doing the parse, so it has the same rules for every command.

I tried to get this to work on tags with spaces (for parity with Timewarrior), but could not get it working before I ran out of time to spend on this. That will have to be left as a future endeavor. I don't personally use spaces.

In the meantime, at least tags with dashes work now, and tags can also use utf8 chars, including as the first/only character, if they wish.

The patch includes a few extra tests using tags with dashes and glyphs.

Note: I did not test sync.

Fixes #3957

[smemsh]

it looks like the test are failing on some python stuff:

  File "/root/code/build/test/tag.test.py", line 612
    self.t(f"add{'\x20+'.join(testtags)} one")
                                             ^
SyntaxError: f-string expression part cannot include a backslash

this works fine on my python. Looks like this was made to work in 3.13.6. I'll dumb it down presently.

[smemsh]

ok, ready.

[djmitche]

This looks good! Please update task(1) to document this, where it describes the +tag|-tag syntax.

Please also make a corresponding update to https://github.com/GothenburgBitFactory/tw.org/blob/master/content/docs/tags.md and anywhere else you see fit in the docs.

Finally, Taskchampion will need updated to correspond -- see https://github.com/GothenburgBitFactory/taskchampion/blob/main/src/task/tag.rs#L8.

[smemsh]

@djmitche so howcome the existing rust code doesn't reject the non-conformant tags coming from taskwarrior? There were no complaints from the storage backend with the C++ patch applied... that rust code you linked looks like it's supposed to bomb if the tag contains dashes.

[djmitche]

I wondered the same thing! The short answer is, that particular code isn't used by Taskwarrior -- Taskwarrior just gets key/value maps from TaskChampion and does whatever it wants with them. The slightly longer answer is that TaskChampion treats any key/value map as valid and won't bomb, but would just ignore invalid tags. So even if someone had tags with characters in them that TC didn't like, and was interoperating with another tool using TC directly, nothing would explode -- they just wouldn't see those tags in the TC-based tool.

[smemsh]

interesting so rather than gate the commit, the pre-commit action does an auto-fixup commit for formatting? I'm supposed to run the hooks on my side before pushing to pre-test this locally right?

[djmitche]

interesting so rather than gate the commit, the pre-commit action does an auto-fixup commit for formatting? I'm supposed to run the hooks on my side before pushing to pre-test this locally right?

That's right. It's an advantage for more one-off PRs where someone might make a minor fix but not be willing to do a second round to fix the formatting.

And, yes, pre-commit install will set up to run this in a Git hook.

[djmitche]

Thanks!!

[djmitche]

👍🏼

[smemsh]

My vote would go for blocking the commit rather than surreptitiously fixing it. I think anyone would do whatever the project requires, if a formatter is required they just need to know. Blocking it in the test actions would serve that purpose.

I only learned it happened accidentally but yet have no objections to enforced style. Well, good to know for next time....

[djmitche]

Let's see how it goes. Let me know if it really starts to get annoying!