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

Add last friday command #197

Merged
merged 1 commit into from Oct 1, 2019
Merged

Add last friday command #197

merged 1 commit into from Oct 1, 2019

Conversation

lzap
Copy link
Contributor

@lzap lzap commented Jun 10, 2019

I report on a daily basis and I miss ability to report last Friday. This should do it, also support next Friday which does not make sense, but it's good to be consistent. Not a pythonist, so take care reviewing.

I wanted to add a more generic "last business day" command but it turns out that it's tough to do business days in plain Python and I do not want to add another dependency because of this. If you want the patch to be Israel-friendly, then I can add also "last sunday" or maybe all the days but that could be little bit noisy.

@AloisMahdal
Copy link
Contributor

BTW, you could hack around it by something like:

did --since `date -d 'last friday' +%Y-%m-%d` --until `date -d 'last friday' +%Y-%m-%d`

or

did_1day() {
    #
    # Report activity for day $1 (in format accepted by date(1) -d)
    #
    local expr=("$@")
    local day
    day=$(date -d "${expr[*]}" +%Y-%m-%d) || {
        echo "could not convert date: '${expr[*]}'" >&2
        return 3
    }
    did --since "$day" --until "$day"
}

and then just call

did_1day last friday

..in the end it would be much more poweful than adding every expression as extra command, since it can do many other tricks.

Or, maybe did could have a way to convert the expression internally?

@lzap
Copy link
Contributor Author

lzap commented Jun 10, 2019

Hmm I did not see "NameError: global name 'FR' is not defined" locally, this is probably defined in the delta module. Anyway, let me know if you want this patch otherwise I'd just drop it given the workaround (nice thanks).

psss added a commit that referenced this pull request Oct 1, 2019
@psss psss self-assigned this Oct 1, 2019
@psss
Copy link
Owner

psss commented Oct 1, 2019

Thanks for the patch. Looks fine. Going to merge after a couple of adjustments in 06bae06. We can extend date selection to be more generic in the future. The last friday seems that it could be a relatively common use case so it's worth to include it now.

@psss psss merged commit 1aba8b3 into psss:master Oct 1, 2019
@lzap lzap deleted the last-friday branch October 3, 2019 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants