printer: deduplicate hyperlink alias names#3103
printer: deduplicate hyperlink alias names#3103ltrzesniewski wants to merge 1 commit intoBurntSushi:masterfrom
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This refactors the way hyperlink aliases are handled by embedding the code in a HyperlinkAlias type. Currently, this also removes a duplicate alias list from the HyperlinkFormat flag in defs.rs, but it is expected to later be used in other features such as shell completion scripts.
a3d5ec1 to
71b8544
Compare
This exports a new `HyperlinkAlias` type in the `grep-printer` crate. This includes a "display priority" with each alias and a function for getting all supported aliases from the crate. This should hopefully make it possible for downstream users of this crate to include a list of supported aliases in the documentation. Closes #3103
|
Thank you for adding this! I ended up re-working this a bit to make the API a little less convenient, but in exchange, it should hopefully be easier to evolve in semver compatible ways if necessary. That is, the revised API doesn't encode as many assumptions. I was somewhat on the fence about accepting this. I'm not really a huge fan of this much ceremony just to avoid a little duplication. In particular, there are other ways to ensure things don't get out of sync, i.e., a CI test or something. But I think overall this is a pretty small and reasonable change, and it might be useful to consumers of |
|
Thanks for accepting this and using it in the other PRs! I thought the Just one nitpick on |
|
Thanks for checking it out! A signed type is probably wiser. But users of the crate can't rely on any specific priority that is given for an alias and there is no way for users to create a new alias or assign a priority. So its meaning is entirely under the control of the implementation. But yeah, I am very grumpy about ripgrep's crates being on crates.io. I kind of regret doing it because it invites so much ceremony. But there are a healthy number of dependents of the I am more lenient about breaking changes than I usually am for more fundamental "ecosystem" crates. But I do still try to avoid them when it isn't too much of a pain otherwise. I'm just used to it after years of doing it with other crates. |
This exports a new `HyperlinkAlias` type in the `grep-printer` crate. This includes a "display priority" with each alias and a function for getting all supported aliases from the crate. This should hopefully make it possible for downstream users of this crate to include a list of supported aliases in the documentation. Closes #3103
This exports a new `HyperlinkAlias` type in the `grep-printer` crate. This includes a "display priority" with each alias and a function for getting all supported aliases from the crate. This should hopefully make it possible for downstream users of this crate to include a list of supported aliases in the documentation. Closes #3103
|
I changed the type of priority from The |
This exports a new `HyperlinkAlias` type in the `grep-printer` crate. This includes a "display priority" with each alias and a function for getting all supported aliases from the crate. This should hopefully make it possible for downstream users of this crate to include a list of supported aliases in the documentation. Closes #3103
This refactors the way hyperlink aliases are handled by embedding the code in a
HyperlinkAliastype.Currently, this removes a duplicate alias list from the
HyperlinkFormatflag indefs.rs, but it should also provide the required features to avoid adding additional duplicates in the following PRs:--hyperlink-formats #3096/cc @ilyagr @okdana