Conversation
The previous implementation would find the prefix that would include `refs/` and the next component. This can be too strict for namespaced references, i.e. `refs/namespaces/*`. This change follows the same approach as the C implementation of Git. It takes the prefix as the whole reference up to the `*`. If there is no `*`, if it starts with `refs/`, take the whole reference. Otherwise, there is no prefix.
FintanH
force-pushed
the
fintohaps/fix-ls-refs-prefixing
branch
from
84d2dae to
485006d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84d2daec19
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// using `agent` information to identify ourselves. | ||
| pub fn new( | ||
| prefix_refspecs: Option<&[gix_refspec::RefSpec]>, | ||
| prefix_refspecs: Option<RefPrefixes>, |
There was a problem hiding this comment.
LsRefsCommand::new now takes Option<RefPrefixes>, but RefPrefixes lives in gix_protocol::ls_refs::function and that module is pub(crate) (and not re-exported), so external callers cannot construct Some(...) anymore. In practice, downstream code can only pass None, which silently removes the public ability to configure ref-prefix filtering through this API and is a source-breaking regression for users that previously passed refspec-based filters.
Useful? React with 👍 / 👎.
The `RefPrefixes` type captures the intent of building a set of `ref-prefix` arguments for the ls-refs protocol. Since refspecs are the natural type that these are built from, as seen in `handshake/mod.rs`, a constructor is provided in the form of `RefPrefixes::from_refspecs`. For other, external API users `RefSpecs::new` and `RefSpecs::extend` are provided.
FintanH
force-pushed
the
fintohaps/fix-ls-refs-prefixing
branch
from
485006d to
69159dd
Compare
1 participant
See discussion at #2429
I didn't quite have the correct environment set up, so I'm not sure if all the tests are passing, but let's see if CI catches anything I messed up :)