GothenburgBitFactory / taskwarrior

Taskwarrior - Command line Task Management
https://taskwarrior.org
MIT License
4.39k stars 298 forks source link

2.6.X: 'eocw' and 'socw' are invalid #3628

Open elig0n opened 1 month ago

elig0n commented 1 month ago
task XXX modify due:eocw
task XXX modify scheduled:eocw

eocw is showing up when ZSH command-completing timed-attributes for example either due: or scheduled: ...

Apply the timed modification

I have checked the v2.6.1 and v2.6.0 refspecs for the string eocw (and socw as well):

$ rg -F eocw
ChangeLog
1914:  Added as well synonyms soww/eoww plus new socw/eocw for calendar weeks.

scripts/zsh/_task
52:    'eocw:End of calendar week' \

scripts/fish/task.fish
389:                             'eocw:End of calendar week' \

doc/man/task.1.in
1102:task ... due:eocw

And they don't show up in .cpp or .h files like eoww does (src/libshared/src/Datetime.cpp and bool Datetime::initialize... functions). But they still appear as valid completions for fish & zsh shells !

smemsh commented 1 month ago

Afaik, there is no "calendar week" shortcut available;eocw is not one of the official expansions. What would it mean? Sunday-based start of week? Monday-based week? ISO week? Those could all be different timepoints.

The one that exists is eow which, I think is always using Monday start, so it represents the last moment in Sunday.

There is a change being discussed in #3623 to allow rc.weekstart to inform the "week-relative" shortcuts, but the effect would be global and no such different concept between a "real week" and "calendar" week currently exists in the source code I think.

(except as an example in task.1 man page, which it probably shouldn't)

elig0n commented 1 month ago

@smemsh

socw/eocw were first added in this commit of Feature #446 by Federico Hernandez (shell completions for them were added later by other contributors).

It says:

    'socw' and 'eocw' refer to the calendar week (starting Sunday/Monday and
    ending Saturday/Sunday)

and they were handled in the now missing src/Date.cpp file whose code also took into consideration weekstart

I hope these functionalities (both the shortcuts and considering config's weekstart) could be reworked into the code perhaps using this reference? https://github.com/GothenburgBitFactory/taskwarrior/issues/3623

felixschurk commented 1 month ago

I think regarding this issue it makes more sense to remove the aliases, as the other abbreviations (which are at least now working or the latest) refer to the next eonw, sonw or previous eopw, sopw week. I updated the documentation a few weeks ago to reflect the latest behavior https://taskwarrior.org/docs/.

Regarding the eocw, socw in some later description, it stated that they refer to "current" week, which would be more consistent with the above abbreviations. And then I assume they got removed as eow is basically also referring to current week.

I think we should definetely update the completion scripts to be up to date with the current implementation.

smemsh commented 1 month ago

I guess if weekstart influenced it, eow and eocw would be different, but there's no need for another week-relative distinction. Even if weekstart is implemented let it have global effect anyways so eow and sow would change along with it.

smemsh commented 1 week ago

@felixschurk I'm curious where you saw socw referring "in some later description" to the "current week" rather than its original meaning "calendar week" ?

My reading of the original code is that sow was an alias for soww (so I guess a "work week" and a "week" were synonymous) and socw was for a "calendar week," which depended on weekstart (could be the same if monday). sow and soww were hardcoded to Monday and eow and eoww were hardcoded to Friday.

I don't know when this changed or the history behind it, but I do note there was some kind of discussion and there exists doc/devel/rfcs/workweek.md which states that:

it has become clear that a 'weekstart' setting, and the notion of a weekend are no longer useful

which seems puzzling to me, as I can definitely see the usefulness of such concepts. It would be great to see the rationale for this statement and the surrounding discussion somewhere.

It makes me reconsider the wisdom of fixing up the week-relative shortcuts to consider weekstart, as I had planned ala #3629/#3623. If the concept of weekstart was discarded after long discussion, and it was decided that Monday should always be the weekstart (this is my preference anyways), maybe the week-relative shortcuts are already correct, and really we should have just fixed week parsing to always use ISO8601 weeks (like they will now if rc.weekstart = monday after #3654) and eliminated Sunday weeks altogether.

But I know that some people wouldn't like that, and there have been several posts -- both here and in the timewarrior project -- asking for weekstart to be honored broadly, so Sunday weeks are clearly still used by some folks, and it's probably not reasonable to remove them.

I still think it doesn't make sense to add the socw/eocw though (unless they are just aliases for sow/eow), because that appears to be what sow/eow are these days (ie, differing from soww/eoww). There were only two different week-concepts before too, but we've [somewhere along the way] shifted around the meaning so that a "week" is now synonymous with a "calendar week" rather than a "work week" as before.

felixschurk commented 1 week ago

I took it from the previous documentation, before I updated it to reflect the current behavior.

To be honest I did not further investigate the code. I mainly updated the documentation to reflect the current behaviour of taskwarrior.

I personally think also that it would not harm to have a working "weekstart" setting.