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

log: Fix limit options not working #507

Merged
merged 3 commits into from
Nov 19, 2021
Merged

Conversation

craigds
Copy link
Member

@craigds craigds commented Nov 19, 2021

Description

In #498 I added some code to pre-parse arguments that we're going to pass to git log, to figure out
which of the arguments were paths. This was necessary for future work (#496).

Unfortunately I failed to account for git's options that take an argument, notably -n 1, and the 1 got treated as a path.

This change fixes the bug for:

  • -n / --max-limit
  • --after / --since / --until / --before
  • --skip
  • --author / --committer
  • --grep

And incidentally adds those options to the --help output:

  -n, --max-count INTEGER         Limit the number of commits to output.
  --skip INTEGER                  Skip INTEGER commits before starting to show
                                  the commit output.

  --since, --after DATE           Show commits more recent than a specific
                                  date.

  --until, --before DATE          Show commits older than a specific date.
  --author PATTERN                Limit the commits output to ones with author
                                  matching the specified pattern (regular
                                  expression)

  --committer PATTERN             Limit the commits output to ones with
                                  committer matching the specified pattern
                                  (regular expression)

  --grep PATTERN                  Limit the commits output to ones with log
                                  message matching the specified pattern
                                  (regular expression)

Since we currently pass all unknown arguments on to git log, there could be other options that fail. Browsing git log --help shows these options that take positional arguments:

I'd like to take a pragmatic approach to fixing these and so have categorised them below:

these ones make no sense in a kart context

(so no need to support/fix them)

  • -L<start>,<end>:<file>, -L:<funcname>:<file>

these ones don't actually accept a separate arg

These ones take an optional argument, so to keep parsing simple, git seems to have required that they be supplied as --opt=value rather than --opt value. Which means they don't actually break in 0.10.6 at all, and don't need fixing.

  • --no-decorate, --decorate[=short|full|auto|no]
  • --branches[=<pattern>] / --tags[=<pattern>] / --remotes[=<pattern>]
  • --pretty[=<format>], --format=<format>
  • --expand-tabs=<n>, --expand-tabs, --no-expand-tabs
  • --notes[=<ref>]
  • --show-linear-break[=<barrier>]

obscure opts that are unlikely to be used

pragmatically it's unlikely anyone's using these, and our rough plan is to stop forwarding unknown options to git log in a near-future release (0.11 or 0.12); ref #493 (comment) - which will break these options anyway. Let's just not re-add them now.

  • --decorate-refs=<pattern>, --decorate-refs-exclude=<pattern>
  • --grep-reflog=<pattern>
  • --min-parents=<number>, --max-parents=<number>, --no-min-parents, --no-max-parents
  • --glob=<glob-pattern>
  • --exclude=<glob-pattern>
  • --no-walk[=(sorted|unsorted)]
  • --encoding=<encoding>
  • --date=<format>

Checklist:

  • Have you reviewed your own change?
  • Have you included test(s)?
  • Have you updated the changelog?

In #498 I added some code to
pre-parse arguments that we're going to pass to `git log`, to figure out
which of the arguments were paths.

This was necessary for future work
(#496). Unfortunately I failed
to account for git's options that take an argument, notably `-n 1`, and
the `1` got treated as a path.

This change fixes the bug for:
* `-n / --max-limit`
* `--after` / `--since` / `--until` / `--before`
* `--skip`
* `--author` / `--committer`
* `--grep`

And incidentally adds those options to the `--help` output.
@craigds craigds force-pushed the fix-log-options-with-args branch from 3618583 to f91bf75 Compare November 19, 2021 01:45
@craigds craigds marked this pull request as ready for review November 19, 2021 01:46
Copy link
Collaborator

@olsen232 olsen232 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be a way to reduce some boilerplate in the repeated lines of "if arg: add args to args" but either way, looks good

@craigds craigds force-pushed the fix-log-options-with-args branch from 7cc5082 to d9847ec Compare November 19, 2021 02:38
@craigds craigds merged commit 09d64a9 into master Nov 19, 2021
@craigds craigds deleted the fix-log-options-with-args branch November 19, 2021 07:36
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

Successfully merging this pull request may close these issues.

2 participants