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

[CLI] Use Git Human-readable date syntax for --lookback analyze flag #4993

Open
erikkerber opened this issue Oct 11, 2023 · 5 comments
Open

Comments

@erikkerber
Copy link

erikkerber commented Oct 11, 2023

The analyze command converts a what looks to be some kind of Go primitive(?)

Whether my interpretation is right or wrong, since it clearly is meant to be used on Git history I'd recommend using Human-readable date syntax directly in place of a date string in --since and allowing users to pass those strings directly.

i.e.

bb analyze --cost --since=1.week
bb analyze --cost --since=6.months --before=3.months

Similar to:

git log --since=1.week
git log --since=6.months --before=3.months
@erikkerber
Copy link
Author

(Sorry this didn't follow the format. I followed a link that circumvented the template splash screen it seems!)

@sluongng
Copy link
Contributor

The current logic leverages flag.Duration in Go standard library, which, in turn, uses time.ParseDuration to parse the duration.

We could potentially use something like https://github.com/markusmobius/go-dateparser/tree/main instead to parse relative dates like git… But I think it would be harder trying to replicate the exact date format from Git 🤔

@erikkerber
Copy link
Author

erikkerber commented Oct 11, 2023

I think the gist of my proposal is those flags can just be 100% pass throughs to Git. More functionality with even less code.

(Probably don't even need to mess with passing through --before)

@sluongng
Copy link
Contributor

I see your point. It should be possible for us to support an alternative pair of flags: --since and --before. When they are used, ignore --lookback (or throw an error) and pass their values directly to git.

The downside is the errors from git could be cryptic to users 🤔

@bduffany WDYT?

@bduffany
Copy link
Member

bduffany commented Oct 12, 2023

Passing the values straight through to git would prevent us e.g. from failing fast if the values are invalid, and also makes the code less portable/flexible since now we're using a git interface as opposed to a more standard go interface (time.Duration).

I think it'd be fine to be more lenient in what we accept for that flag value though, e.g. use a little regex like ^(\d+)\.(day|week|month|year)s?$ and then fall back to time.ParseDuration if the regex doesn't match.

I also like the separate --since / --before flag approach you mentioned, but it might be preferable to call them --git_since / --git_before since they're git-specific, and also this makes it more clear that they're git passthroughs & follow the git time format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants