loosen tag restriction to allow all non-whitespace, after the first char #3961
Conversation
|
it looks like the test are failing on some python stuff: this works fine on my python. Looks like this was made to work in 3.13.6. I'll dumb it down presently. |
…GothenburgBitFactory#3957) Instead of walking the string and stopping at the first non-isIdentifier() character (see Lexer::isIdentifierStart(), isIdentifierNext() and isSingleCharOperator()), walk all the way to the end of the word. This allows even punctuation and other characters to be used in tags. We still need to use isIdentifierStart() for the first character, to disambiguate it from a negative number or subtraction. Apparently there is no command context available, so the parser cannot "know" whether it's doing a "task calc" and have different parse rules. The lexing seems to happen before breaking the arguments down into commands for dispatch.
smemsh
force-pushed
the
tagword-liberalize
branch
from
de3108a to
6a15239
Compare
for more information, see https://pre-commit.ci
|
ok, ready. |
There was a problem hiding this comment.
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.
|
@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. |
|
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. |
2 participants
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