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

FR: support a(n overridable) way to ignore certain revisions for annotate etc. #4990

Open
chriskrycho opened this issue Nov 28, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@chriskrycho
Copy link
Contributor

Is your feature request related to a problem? Please describe.

git blame allows you to ignore specific revisions, either by passing ----ignore-revs-file <file> or by setting the blame.ignoreRevsFile option in config. This is handy for tracing the history of a file while skipping over things like applying autoformatting or file renames (assuming reasonable move detection, in the latter case). Presently jj file annotate does not have any equivalent capability, so in the case of a move or a formatting command, it will always end

(Note: this was discussed a bit in #2988 starting here.)

Describe the solution you'd like

In broad strokes, it would be good to have a good default for a file which can specify changes or commits to ignore, along with the ability to specify a different file via config (presumably per-repo) and CLI. The jj file annotate command should presumably also accept a flag to override that if present, and blame everything, so that if someone wants to see that, they can.

In Git, the file does not necessarily have to be checked in, and in fact can be ignored implicitly or explicitly, so it can be purely local to a single developer’s machine, but it also can be checked in, which is nice for shared projects where most developers will want to ignore the commits where the repo got restructured, or where they ran Prettier on every JS file, etc.

Describe alternatives you've considered

The obvious alternative is to do nothing. Another would be to approach it from a different angle, possibly including the ability to interactively ignore certain commits while doing an annotate operation. That seems substantially more complicated, and I suspect most of the machinery required to do that would also support the file-based version.

Additional context

N/A

@arxanas
Copy link
Contributor

arxanas commented Nov 28, 2024

Arguably, the feature should maybe not be specifically a file with commits to ignore, but simply a revset of commits to ignore, and then we should add a general way to load a revset from a file.

  • Then you can do extra ad-hoc specifications like "ignore the default commits and also anything that touched file X".
  • It would also work well with future extensions, like if we tagged commits with metadata like "this was a formatting commit" or "tests failed for this commit", without needing to adjust the design to accommodate it.
  • One downside is that then there's even less of a convention about what to name the special file with commits to ignore, but I personally think it's not a huge deal.
    • There might also be a desire to generalize away from a file as well, like a way to define/configure revsets inside the repo, such as trunk() or other custom ones.
    • I guess if we're already relying on conventions like trunk(), then we could certainly also have a conventional revset that defines commits to ignore by default?

@joyously
Copy link

I guess if we're already relying on conventions like trunk(), then we could certainly also have a conventional revset that defines commits to ignore by default?

It sounds dangerous (or at least confusing) to use the term "ignore", when that is well-known for files. Isn't there already a revset function like hidden()?

@emilazy
Copy link
Contributor

emilazy commented Nov 28, 2024

Note that GitHub respects .git-blame-ignore-revs by default, so I think we should support that out of the box for the same reason as .gitignore. (It’s strange that git(1) doesn’t use it unless configured, but… well, that’s Git for you.)

@yuja
Copy link
Contributor

yuja commented Nov 28, 2024

Arguably, the feature should maybe not be specifically a file with commits to ignore, but simply a revset of commits to ignore,

fwiw, it's equivalent to hg annotate --skip REVSET.

@arxanas
Copy link
Contributor

arxanas commented Nov 28, 2024

It sounds dangerous (or at least confusing) to use the term "ignore", when that is well-known for files. Isn't there already a revset function like hidden()?

I wouldn't use the term "ignore" in this case; I just meant to suggest that there could be a conventional revset for the concept itself.

@martinvonz
Copy link
Member

The obvious alternative is to do nothing. Another would be to approach it from a different angle, possibly including the ability to interactively ignore certain commits while doing an annotate operation.

That's what I was going to say. I we have a TUI for jj file annotate, then that TUI should make it easy to select "annotate at parent commit".

I'm not strongly against support for annotating within a revset. I just feel like it's often (but not always) a workaround.

@chriskrycho
Copy link
Contributor Author

@arxanas I think you’re absolutely right that the feature should be more general and could be defined multiple ways that way. It probably should have been obvious to me that it’s just another revset at the end of the day! Having a file just becomes a useful way to check in a revset. Duh. 😅 Still haven’t fully internalized some of the new things the jj approach unlocks, it turns out!

@martinvonz yeah, a TUI would be great. Presumably the lower-level capability needs to be there even so, because gg and editors and forged other such tools will also want to be able to do the same.p, which is why I put it in the alternatives section.

@martinvonz
Copy link
Member

@martinvonz yeah, a TUI would be great. Presumably the lower-level capability needs to be there even so

What's the lower-level capability? I meant that the TUI (and other interactive UIs) should provide an option to annotate the file at the parent of the commit that last modified a given line. So if you're looking at the annotated output for a file and it says that the line you care about was last modified in a code-formatting commit, then you just press alt-P or whatever to annotate the parent is that commit.

My assumption is that the goal of annotating the file is so you can figure out why the code looks the way it does. If your use case is more related to finding recent authors to send a patch for review for, then my suggestion is not that useful.

@chriskrycho
Copy link
Contributor Author

@martinvonz I think we’re broadly getting at the same thing, but a lot of times it is not an interactive UI, or it’s only sort-of-interactive (like a forge view, or honestly like most editors’ implementations today), and moreover I don’t really ever want to see autoformatting or repo-restructure commits “by default”. I guess what I’m saying is an interactive view sounds nice, but I also want it to be easy to do without interactivity.

@PhilipMetzger PhilipMetzger added the enhancement New feature or request label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants