feat: add jujutsu walker #601
Conversation
delafthi
changed the title
delafthi
force-pushed
the
push-rtxoonqmxkxy
branch
2 times, most recently
from
2dd1b91 to
60b2c48
Compare
|
Jujutsu tests in |
delafthi
force-pushed
the
push-rtxoonqmxkxy
branch
8 times, most recently
from
84f58f0 to
c6d51bc
Compare
There was a problem hiding this comment.
The approach here looks good to me.
In the future, it could make sense to create some abstraction over git and jj, but IMO that's not warranted here. Copying and tweaking is good, and the tests you're adding are super valuable if we ever feel the need to do a future refactor.
| @@ -329,6 +330,21 @@ func determineTreeRoot(v *viper.Viper, cfg *Config, logger *log.Logger) error { | |||
| } | |||
| } | |||
|
|
|||
| // attempt to resolve with jujutsu | |||
There was a problem hiding this comment.
If walk == auto, do we first attempt to resolve with git, and then attempt to resolve with jujutsu? That feels silly to me: we should not keep searching after we find a tree route. In other words, guard this with cfg.TreeRoot == "" (as we do below).
I suspect there's a flatter refactor of all this that looks less like the current case with a big default block and is instead:
- if no tree root, search with tree root file
- if (still) no tree root, search with tree root cmd
- if (still) no tree root (and walk is auto or git), search with git
- if (still) no tree root (and walk is auto or jujutsu), search with jujutsu
- if (still) no tree root, use the directory containing the config file
If you do opt to make this refactor, please split it into a separate initial commit that we could merge first.
There was a problem hiding this comment.
Currently, I added cfg.TreeRoot == "" as a guard:
if cfg.TreeRoot == "" && (cfg.Walk == walk.Auto.String() || cfg.Walk == walk.Jujutsu.String()) {
logger.Infof("attempting to resolve tree root using jujutsu: %s", jujutsu.TreeRootCmd)
...
delafthi
force-pushed
the
push-rtxoonqmxkxy
branch
2 times, most recently
from
1f31b83 to
4b4c4f5
Compare
- added jujutsu module similar to the git module, which provides the `IsInsideWorktree` function - added jujutsu walker, with the following differences to the git walker - the list command does not update the index. thus, new files are not listed, the user has to add them to the index first by running a `jj` command - added jujutsu walker test - added jujutsu walker root test - adapted config and docs
delafthi
force-pushed
the
push-rtxoonqmxkxy
branch
from
4b4c4f5 to
de5d307
Compare
|
Additionally, I rebase unto latest main to fix the failing tests in |
|
I'm not sure why the current tests are failing. The difference is always one file that gets additionally formatted. Also, I get the exact same result when building on |
Removed unnecessary unquoting logic for lines (file paths) starting with a quote.
delafthi
force-pushed
the
push-rtxoonqmxkxy
branch
from
811c171 to
6cfbdf0
Compare
|
@delafthi I think what you're seeing is a brittle test. I've been seeing it periodically and trying to get to the bottom of it with no joy 😞 I'll try to find some time to review this week. |
|
@Enzime can you give this a review since I'm really not familiar with jujutsu |
This PR introduces a new Jujutsu walker to numtide/treefmt, modeled after the existing Git walker but tailored to Jujutsu repositories. Key differences include:
jjcommands.This addition enables proper handling of Jujutsu ignore rules without requiring co-located Git repos, improving integration and file traversal accuracy for Jujutsu users.
I lack Go experience and primarily copied, and adapted the existing Git walker by adjusting commands. I would welcome guidance or collaboration to develop a more idiomatic and robust implementation.