Support pathspecs in --cut #134
Conversation
gitrevise/utils.py
Outdated
| index.git("reset", "--patch", final_tree.persist().hex(), "--", ".", stdout=None) | ||
| index.git( | ||
| "reset", | ||
| "--patch", | ||
| final_tree.persist().hex(), | ||
| "--", | ||
| *pathspecs if pathspecs else ".", | ||
| stdout=None, | ||
| ) |
There was a problem hiding this comment.
Does this work from a subdirectory (when pathspecs (or ".") are relative to the current directory)?
I'm actually using this daily, and happened to try it from a subdirectory now. It works if I use a repo-absolute path from within the subdirectory. But looking at the code, I'm surprised that that "." doesn't otherwise mean the subdirectory (not that I think it should).
Idea: You could make the tests run from a subdirectory to cover this aspect too.
There was a problem hiding this comment.
Is there an alternative to git reset?
This appears to be a quirk of git reset: Its pathspec arguments are not relative. Although (according to gitglossary(7)) it is command defined whether they are relative, I think relative is the only sensible interpretation: It is what your shell completes, whereas a toplevel relative pathspec is denoted by for example a :/ prefix. So if we want that, we would need to not use git reset, or work around it. It also explains how the dot becomes the toplevel directory.
Fixing it the obvious way (by prefixing the current directory) would necessitate parsing the pathspec, which looks a bit undesirable to have to deal with: Not just parsing wise (even parsing the short form of the magic character prefix requires knowing the full set of magic characters), but we need to understand it, to know whether the path is supposed to be toplevel relative or not.
There was a problem hiding this comment.
This appears to be a quirk of git reset: Its pathspec arguments are not relative
No. According to my testing they are treated like any other pathspec (relative unless prefixed with :/ or /).
If I run
mkdir a b
touch a/1 b/2
git add a b
cd a
git reset --patch -- 1that works as expected (only includes a/1, not b/2).
Now if I cancel the last command and do
git reset --patch -- .it will prompt to reset anything inside a/, so effectively the same.
It works if I use a repo-absolute path from within the subdirectory.
This is surprising.
Also, relative paths don't work (they should).
This is a bug in git-revise.
By default it runs git reset with the Git worktree as working directory.
if cwd is None:
cwd = getattr(self, "workdir", None)Because of this, our -- . doesn't make a difference today (it is confusing though).
Once we fix this however, it will cause deviant behavior from git reset.
I think we should remove it.
I'm not sure why it was added in 74c59ad (Improve performance and UX for --cut, 2019-07-21), perhaps by accident.
There was a problem hiding this comment.
I agree. You found the right problem.
I had a go at running git from cwd (which indeed makes the pathspecs work from a subdirectory):
https://github.com/anordal/git-revise/commits/run-git-from-cwd
There was a problem hiding this comment.
I opted for a minimal fix here but I agree that it might be worth to change the default to keep the working directory, instead of using the work tree root.
However we need to be careful, your patch changes the behavior of our call to git ls-files. Not sure if for better or worse.
gitrevise/tui.py
Outdated
| @@ -36,6 +36,9 @@ def build_parser() -> ArgumentParser: | |||
| nargs="?", | |||
| help="target commit to apply fixups to", | |||
| ) | |||
| parser.add_argument( | |||
| "pathspecs", nargs="*", help="make --cut select only from matching files" | |||
There was a problem hiding this comment.
probably should set it up to only be allowed when --cut is used? I think clap supports this kind of thing
There was a problem hiding this comment.
yeah that was a regression, fixed
Sometimes I want to extract a small number of changes from a commit that touches loads of files. Since the cut mode prompts about every single file, there is no convenient way to do this. Teach --cut about pathspec arguments. This allows to use git revise --cut my-commit -- path/to/my/file a to extract to a separate commit all changes in the given file. In future we could support pathspecs in other subcommands.
krobelus
force-pushed
the
cut-by-pathspec
branch
from
0995c3b
to
9d8d89e
Compare
Sometimes I want to extract a small number of changes from a commit
that touches loads of files. Since the cut mode prompts about every
single file, there is no convenient way to do this.
Teach --cut about pathspec arguments. This allows to use
to extract to a separate commit all changes in the given file.
cc @Munsio