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

Filter to only include hunks? #23

Closed
ErichDonGubler opened this issue Apr 30, 2018 · 17 comments · Fixed by #940
Closed

Filter to only include hunks? #23

ErichDonGubler opened this issue Apr 30, 2018 · 17 comments · Fixed by #940
Assignees
Labels
feature-request New feature or request

Comments

@ErichDonGubler
Copy link

First: congrats on the release, and props for creatively putting together the pieces that make this work!

Anyway: It'd be really neat if this could be used as an alternative to the default pager in Git -- this is REALLY nice-looking, and I'd love to use this as a pager. Would you be open to a PR or implementing yourself a feature to filter out everything that's not a hunk in a diff? I'm not sure if this should be a special way to process diff/patch files or just integrated with libgit2 directly -- the latter is probably easier, since you're already using libgit2 as a dependency.

@sharkdp
Copy link
Owner

sharkdp commented Apr 30, 2018

Thank you for the feedback!

Nice idea! I actually was on a similar track here: #12 (see --context option).

Would you be open to a PR

Definitely! However, maybe it would be worth to first discuss some details here in this ticket?

For example, how would the command line interface (subcommand, option/flag) for that feature look like?

I'm not sure if this should be a special way to process diff/patch files or just integrated with libgit2 directly -- the latter is probably easier, since you're already using libgit2 as a dependency.

Yes, libgit2 sounds like the better option to me.

@sharkdp sharkdp added feature-request New feature or request help wanted Extra attention is needed labels May 1, 2018
@ErichDonGubler
Copy link
Author

ErichDonGubler commented May 1, 2018

I think that at this point a single flag or subcommand would be appropriate -- the assumption with a libgit2-based diff is that you're operating inside of a Git repository.

Requirements I'm confident should be part of this

This flag/subcommand (hereafter referred to as diff, which is definitely subject to bikeshedding):

  • will fail the entire command if not inside of a Git repo
  • should take a reasonable subset of options that map to common git diff invocations; some example invocations might be:
    • bat diff would display all unstaged changes, minus untracked files (a la git diff)
    • bat diff cached would display all staged changes (a la git diff --cached)
      • Alternative invocations for this use case could look like bat --diff --cached

Unresolved questions

  • Should this be a subcommand or a flag?

    • Flags would be unambiguous, but a subcommand might look more ergonomic (git-esque, in this case).
    • If a subcommand is desired, then maybe the following design would work:
      • bat print or some similar subcommand would be the default when only file paths are provided. For instance, given the following file structure: foo:
        $ tree
        .
        ├── diff
        ├── foo.txt
        └── print
        
          ...then:
        
        • bat foo.txt would be equivalent to today's functionality, inferred to be bat [print] foo.txt.
        • bat print would be ambiguous and return an error reporting that a file and a subcommand exist with that name. This can be resolved by using bat print print. This case could also just print a warning to stderr, since in this case the intent to print a file is NOT ambiguous.
        • bat diff would likewise report an ambiguity, as with print. This would have to be a hard error, and resolved with either bat print diff or bat diff --, depending on the intended subcommand.
      • Alternatively, bat <file> shorthand could simply be removed and get rid of the ambiguous cases in the above design.
  • Should bat diff be a filter of the format we get from bat print, or more of a diff pager?

    • The difference is that a filter would only show diff number column marks, while a diff pager would actually show the deleted/added lines a la git diff.
    • IIUC, treating this flag as a filter more closely matches the spirit of the --context idea in the issue you linked.

@sharkdp
Copy link
Owner

sharkdp commented May 2, 2018

@ErichDonGubler Thank you very much for writing this up! It might take me a few days before I get back to you on this.

@sharkdp
Copy link
Owner

sharkdp commented May 14, 2018

What I don't really like about adding a diff subcommand (or option) is that we open up a completely new field with that functionality. While we are not truly following the UNIX "do one thing and do it well" philosophy with bat, it is mainly a program that is intended to display file contents.

So here is a slightly different proposal that I would like to discuss:

  • Add a --diff-context=<lines> option (didn't come up with a better name for now), which, when enabled, would only show <lines> lines of context around modified parts of the specified files (the default would be "--diff-context=infinite").
  • Write a tool to provide git diff-like functionality. This could be as simple as
    git diff --name-only | xargs bat --diff-context=3

Edit: changed --hunk-context to --diff-context

@ErichDonGubler
Copy link
Author

Your first bullet point sounds like a waaay-streamlined version of what I just proposed. I would definitely choose that over what I suggested. :)

I would also vote for only doing the first bullet point as a first iteration -- I'd be happy with that, and I don't know how straightforward it would be to highlight things without the full file.

@sharkdp sharkdp added this to the v0.5.0 milestone Aug 18, 2018
@sharkdp sharkdp removed the help wanted Extra attention is needed label Aug 25, 2018
@sharkdp sharkdp removed this from the v0.6.0 milestone Aug 27, 2018
@bendem
Copy link

bendem commented Sep 7, 2018

Since we are speaking about this and it's being considered, is there any plan for also supporting diff-highlight type of highlighting?

@sharkdp
Copy link
Owner

sharkdp commented Oct 1, 2018

@bendem Could you elaborate what that means?

@sharkdp
Copy link
Owner

sharkdp commented Oct 1, 2018

I think my proposal above should be rather straightforward to implement.

I think we should first (possibly in a separate PR) update the --line-range option to be accepted multiple times.

In a second step, we would then use the Git modifications and the value given in the --diff-context option to generate a list of line-ranges that could be processed by the already existing line-range functionality

@bendem
Copy link

bendem commented Oct 3, 2018

@bendem Could you elaborate what that means?

I mean that git supports highlighting inside of lines, which I don't think bat does yet. That would be an awesome feature to have.

diff-highlight
screen shot 2018-10-03 at 09 35 11

no diff-highlight
screen shot 2018-10-03 at 09 36 31

@sharkdp
Copy link
Owner

sharkdp commented Oct 19, 2018

@bendem Thank you for your feedback. This is not really related to this ticket. I don't currently plan to implement inline-diffs and I think that would be quite a substantial feature. If you think this should be discussed, please open a new ticket.

sharkdp added a commit that referenced this issue Oct 19, 2018
sharkdp added a commit that referenced this issue Oct 20, 2018
@sharkdp
Copy link
Owner

sharkdp commented Oct 20, 2018

I think we should first (possibly in a separate PR) update the --line-range option to be accepted multiple times.

That first part has been implemented in #368

@fdcds
Copy link

fdcds commented Nov 22, 2019

@ErichDonGubler: Are you aware of https://github.com/dandavison/delta, which implements a pager for git diffs?

@sharkdp
Copy link
Owner

sharkdp commented Nov 24, 2019

We are aware of delta. That's why we added it to the README.

@sharkdp sharkdp added this to the v0.14 milestone Apr 11, 2020
@sharkdp sharkdp removed this from the v0.14 milestone Apr 21, 2020
sharkdp added a commit that referenced this issue Apr 23, 2020
This adds a new `--diff` option that can be used to only show lines
close to Git changes (added/removed/modified lines). The amount of
additional context can be controlled with `--diff-context=N`.

closes #23
@sharkdp sharkdp mentioned this issue Apr 23, 2020
@sharkdp
Copy link
Owner

sharkdp commented Apr 23, 2020

It's been almost two years, but I finally found the time to implement this. See #940 for details and screenshot.

sharkdp added a commit that referenced this issue Apr 23, 2020
This adds a new `--diff` option that can be used to only show lines
close to Git changes (added/removed/modified lines). The amount of
additional context can be controlled with `--diff-context=N`.

closes #23
sharkdp added a commit that referenced this issue Apr 23, 2020
This adds a new `--diff` option that can be used to only show lines
close to Git changes (added/removed/modified lines). The amount of
additional context can be controlled with `--diff-context=N`.

closes #23
sharkdp added a commit that referenced this issue Apr 23, 2020
This adds a new `--diff` option that can be used to only show lines
close to Git changes (added/removed/modified lines). The amount of
additional context can be controlled with `--diff-context=N`.

closes #23
@ErichDonGubler
Copy link
Author

@sharkdp: Not enough emojis to express my excitement. :)

sharkdp added a commit that referenced this issue Apr 24, 2020
This adds a new `--diff` option that can be used to only show lines
close to Git changes (added/removed/modified lines). The amount of
additional context can be controlled with `--diff-context=N`.

closes #23
@Mouvedia
Copy link

@sharkdp can you publish a new release?

@sharkdp
Copy link
Owner

sharkdp commented Apr 25, 2020

Released in v0.15.0.

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

Successfully merging a pull request may close this issue.

5 participants