worktree stack supports excludes #397
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It doesn't work as one would expect due to it wanting to match relative paths only. Thus it's better to spare folks the surprise and instead use `wildmatch()` directly. It works the same, but doesn't have certain shortcuts which aren't needed for standard matches anyway.
It caches as much as it can for each pattern list. Many more tests are needed, but that will come in with tests that compare git with our implementation.
Also allow to read from bytes directly, which happens if the file is read from the index/odb.
Also it turns out that we can't really unify the pattern list creation yet, unless we'd add parse errors to the trait which maybe is preferable over duplicating some code.
…which haven't been covered by tests and which show that the current implementation isn't sufficient. It also makes clear that the match styles of git-attributes and git-ignore are quite different. It's unclear why, even though the way both algorithms work are quite different. So have to take a step back and find a way to figure it out.
The failing tests are due to shortcomings in git-glob pattern matching right now.
…#301) The stack-based matching is responsible for excluding entire directories which is something the git-attributes based matching doesn't do.
…but it feels a little like a hack. Maybe this should tell us that we need to reorganize code elsewhere?
)" This reverts commit d0c8407.
Very similar to `string()`, but as path, whose query can never fail.
XDG home is still needs to be made accessible, along with a few other improvements.
…ery` (#301) Details are still to be fixed, and reading from stdin should be implemented one day. Issue right now is that the source path is normalized for some reason, let's see where that happens.
If and only if it canonicalized the source path. That way, users will still get a familiar path. This is due to `parent()` not operating in the file system, which otherwise would be equivalent to `..`, but that's not how we work. Maybe we should overhaul the way this works to use `../` instead and just 'absoluteize' the path later (std::path::absolute()) is on the way for that.
It doesn't reproduce on a local VM though.
…301) Fixes windows CI issue, I hope… .
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Feb 15, 2025
The `to_unix_separators` and `to_windows_separators` functions in `gix_path::convert` had TODO comments saying they should use the `path-slash` crate "to handle escapes". These comments were added as part of e4f4c4b (GitoxideLabs#397) but the context there and in the broader related issue GitoxideLabs#301 does not seem to clarify the basis for this. It is not really clear what handling escapes would entail here, and it seems like there is not a way to do it without substantially changing the interface of these conversion functions in `gix-path`, which currently take a single argument representing a path and return a single string-like value also representing a path. If escape sequences appaer in the input to such a path conversion function, it would not have a way to know if they are meant literally or as escape sequences. (An analogous concern applies if a function is to add escape sequences in its return value; it would have no way to know if the caller expects them.) Furthermore, while `path-slash` can convert some `\` paths to use `/` instead, it does not appear to do anything related to handling escape sequencs or distinguishing which occurrences of `\` or any other character may be intended as part of an escape sequence. Its documentation does prominently mention that `\` in escape sequnces should not be converted to `/`: > On Unix-like OS, the path separator is `/`. So any conversion is > not necessary. But on Windows, the file path separator is `\`, > and needs to be replaced with `/` for converting the paths to > "slash paths". Of course, `\`s used for escaping characters > should not be replaced. But it looks like the part about `\` characters used for escaping is meant as advice on how and when to use `path-slash`, rather than meaning that `path-slash` would itself be able to distinguish between `\` characters meant as directory separators and `\` characters that perform quoting/escaping.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Feb 15, 2025
The `to_unix_separators` and `to_windows_separators` functions in `gix_path::convert` had TODO comments saying they should use the `path-slash` crate "to handle escapes". These comments were added as part of e4f4c4b (GitoxideLabs#397) but the context there and in the broader related issue GitoxideLabs#301 does not seem to clarify the basis for this. It is not really clear what handling escapes would entail here, and it seems like there is not a way to do it without substantially changing the interface of these conversion functions in `gix-path`, which currently take a single argument representing a path and return a single string-like value also representing a path. If escape sequences appear in the input to such a path conversion function, it would not have a way to know if they are meant literally or as escape sequences. (An analogous concern applies if a function is to add escape sequences in its return value; it would have no way to know if the caller expects them.) Furthermore, while `path-slash` can convert some `\` paths to use `/` instead, it does not appear to do anything related to handling escape sequences or distinguishing which occurrences of `\` or any other character may be intended as part of an escape sequence. Its documentation (https://docs.rs/path-slash/latest/path_slash/) does prominently mention that `\` in escape sequences should not be converted to `/`: > On Unix-like OS, the path separator is `/`. So any conversion is > not necessary. But on Windows, the file path separator is `\`, > and needs to be replaced with `/` for converting the paths to > "slash paths". Of course, `\`s used for escaping characters > should not be replaced. But it looks like the part about `\` characters used for escaping is meant as advice on how and when to use `path-slash`, rather than meaning that `path-slash` would itself be able to distinguish between `\` characters meant as directory separators and `\` characters that perform quoting/escaping.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A way to obtain a set of attributes given a path, efficiently.
Each path we handle needs one lookup there during checkout, thus
it must be lazy and per thread.
git seems to implement it as stack which is efficient if similar paths are handled together,
and then drops information as it is no longer needed. This seems like the way to go
as we never have to optimize for random access.
Tasks
gix repo exclude query