Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
elig0n opened this issue Sep 22, 2024 · 6 comments
Open

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

elig0n opened this issue Sep 22, 2024 · 6 comments
Labels
topic:autocomplete topic:time Issues relating to time

Comments

@elig0n
Copy link

elig0n commented Sep 22, 2024

  • What command(s) did you run?
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: ...

  • What did you expect to happen?

Apply the timed modification

  • What actually happened?
'eocw' is not a valid date in the 'Y-M-D' format.

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
Copy link
Contributor

smemsh commented Sep 23, 2024

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
Copy link
Author

elig0n commented Sep 24, 2024

@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? #3623

@felixschurk
Copy link
Collaborator

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.

@djmitche djmitche added the topic:time Issues relating to time label Sep 27, 2024
@smemsh
Copy link
Contributor

smemsh commented Sep 28, 2024

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
Copy link
Contributor

smemsh commented Oct 22, 2024

@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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:autocomplete topic:time Issues relating to time
Projects
Status: Backlog
Development

No branches or pull requests

4 participants