Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Replace package manager URL from yazi-rs/plugins#full-border to yazi-rs/plugins:full-border #1471

Closed
sxyazi opened this issue Aug 12, 2024 · 16 comments · Fixed by #1490
Labels
RFC Request for Comments

Comments

@sxyazi
Copy link
Owner

sxyazi commented Aug 12, 2024

To make plugin maintenance easier, Yazi's package manager supports monorepos — where multiple smaller plugins are distributed within the same Git repository. For this reason, the # symbol was introduced to identify subdirectories within the repo, such as yazi-rs/plugins#full-border, which corresponds to https://github.com/yazi-rs/plugins/tree/main/full-border.yazi.

However, since the release of Yazi 0.3, we've received several reports ([1], [2]) from users over the past few days that # is treated as a special character in their shell, which requires escaping. This is often due to certain plugins being enabled.

As a result, ya pack -a yazi-rs/plugins#full-border has to be written as ya pack -a "yazi-rs/plugins#full-border", which conflicts with the simple design philosophy of ya pack.

When users are typing commands rather than copying them, they would need to manually add quotation marks, and this also necessitates that all community plugin READMEs properly escape these characters. Otherwise, users might face inconvenience when trying to install the plugins. This RFC aims to permanently address this issue before ya pack becomes more widely used.

Therefore, this RFC proposes finding another symbol to replace it. Currently, / seems to be a strong alternative. However, this might conflict with PR #1325, which would allow users to use arbitrary URLs, such as: ya pack -a https://gitlab.com/<owner>/<repo>.

The concern here is that we cannot guarantee that all Git providers use this two-level URL structure, for example something like ya pack -a https://my-awesome-domain.com/web/<owner>/<repo> might be recognized as a subdirectory under the repo rather than the repo itself, this was the original reason for choosing #, as it is not an essential part of the URL but rather used by browsers as an anchor, meaning we can safely ignore it.

@sxyazi sxyazi added feature New feature request RFC Request for Comments and removed feature New feature request labels Aug 12, 2024
@sxyazi
Copy link
Owner Author

sxyazi commented Aug 12, 2024

Pinging @GianniBYoung @pirafrank @uncenter for their wisdom

@uncenter
Copy link
Contributor

I think supporting a specific few Git host URL patterns is feasable, which would allow supporting plugins on different branches as well as subdirectories. The two issues with that though are self hosted Git instances would be harder to deal with, and this doesn't solve the "shorthand" syntax. I'm not sure about using any symbols for the command- I'm a fan of ya pack -a yazi-sr/plugins -d smart-filter -b some-branch rather (using flags or just optional arguments instead of a custom syntax).

@pirafrank
Copy link

pirafrank commented Aug 12, 2024

I think supporting a specific few Git host URL patterns is feasable, which would allow supporting plugins on different branches as well as subdirectories. The two issues with that though are self hosted Git instances would be harder to deal with, and this doesn't solve the "shorthand" syntax. I'm not sure about using any symbols for the command- I'm a fan of ya pack -a yazi-sr/plugins -d smart-filter -b some-branch rather (using flags or just optional arguments instead of a custom syntax).

I may agree to custom options, yet in addition to custom option to show in 'help', it is very common to have a short syntax with a URL-like reference for widely used Git hosting platforms as GitHub and GitLab. : may help in this to recall the known scp syntax, e.g.

owner/repo:some/subdir

with .yazi still omitted.

And I agree that such reference should also take into account a branch different then the default one, it may become:

owner/repo@feature/non-default-branch:some/subdir

with @ which should be a safe char as well.

@sxyazi
Copy link
Owner Author

sxyazi commented Aug 12, 2024

I'm a fan of ya pack -a yazi-sr/plugins -d smart-filter -b some-branch rather (using flags or just optional arguments instead of a custom syntax).

I think ya pack -a -d -b is more likely to cause confusion because there are too many abbreviations that don't convey the actual behavior - if it's just ya pack -a, I know it's an "add" action, but if we introduce multiple flags, they would be on the same level, and their order wouldn't be fixed.

This could lead to various combinations, such as:

  • ya pack -a yazi-sr/plugins -d smart-filter -b some-branch
  • ya pack -d smart-filter -a yazi-sr/plugins -b some-branch
  • ya pack -b some-branch -d smart-filter -a yazi-sr/plugins

This also wouldn't be easy to implement; we'd need to find a way to enable the -d and -b options only when the user passes -a. And I'm not sure how to describe them in ya pack --help, which currently looks like this:

❯ ya pack --help
Manage packages

Usage: ya pack [OPTIONS]

Options:
  -a, --add <ADD>  Add a package
  -i, --install    Install all packages
  -l, --list       List all packages
  -u, --upgrade    Upgrade all packages
  -h, --help       Print help

@sxyazi
Copy link
Owner Author

sxyazi commented Aug 12, 2024

: may help in this to recall the known scp syntax

with @ which should be a safe char as well.

Are : and @ not special characters in your shell (at least)? If that's the case, I think this could work, and it seems to apply to SSH as well:

  • ya pack -a yazi-rs/plugins:full-border
  • ya pack -a ssh://git@github.com:22/yazi-rs/plugins:full-border
  • ya pack -a ssh://git@github.com:22/yazi-rs/plugins:feature/non-default-branch@full-border or ya pack -a ssh://git@github.com:22/yazi-rs/plugins:full-border@feature/non-default-branch?

@pirafrank
Copy link

Are : and @ not special characters in your shell (at least)? If that's the case, I think this could work, and it seems to apply to SSH as well:

They should work across many shells, the same way SSH commands do.

  • ya pack -a ssh://git@github.com:22/yazi-rs/plugins:feature/non-default-branch@full-border or ya pack -a ssh://git@github.com:22/yazi-rs/plugins:full-border@feature/non-default-branch?

Second option, imo.

I'd always mark the start of subpath with : for consistency with mentioned ssh and scp.

@ would mark the start of non-default branch name, and this block would eventually be appended to the string.

@sxyazi
Copy link
Owner Author

sxyazi commented Aug 12, 2024

@ would mark the start of non-default branch name, and this block would eventually be appended to the string.

Nice! Yazi has already restricted plugin names to only include letters and hyphens, which makes handling them much easier, we can safely extract everything after the @ as the branch name, just in case the branch name contains an @. I just tested it, and Git does indeed allow branch names with an @ lol.

Thank you so much for your input. I think this could be the final solution, as long as no other issues come up during implementation. I'll try to implement the : part first in the next version and provide a gradual transition plan — the existing # will still be available but will show a warning to encourage users to adopt the new format to help avoid breaking changes.

@uncenter
Copy link
Contributor

Sounds good to me as well!

@sxyazi sxyazi changed the title [RFC] Replace package manager URL from yazi-rs/plugins#full-border to yazi-rs/plugins/full-border [RFC] Replace package manager URL from yazi-rs/plugins#full-border to yazi-rs/plugins:full-border Aug 12, 2024
@GianniBYoung
Copy link

Throwing in my two cents; I like the simplicity of using : over remembering and understanding cli flags

@uncenter
Copy link
Contributor

Should we maybe open a new tracking issue for adding support for branches? I see that you renamed commit to rev in #1490, looks like moving toward that? #1490 also creates conflicts with #1325 and with it being open for the last 3 weeks I'm not interested in resolving them again. Feel free to pull in my changes though.

@pirafrank
Copy link

Should we maybe open a new tracking issue for adding support for branches? I see that you renamed commit to rev in #1490, looks like moving toward that? #1490 also creates conflicts with #1325 and with it being open for the last 3 weeks I'm not interested in resolving them again. Feel free to pull in my changes though.

Hi @uncenter, yes option to express a branch name to checkout was part of the efforts of this RFC. You can read it's mentioned in comments above with the @ notation.

@uncenter
Copy link
Contributor

Hi @uncenter, yes option to express a branch name to checkout was part of the efforts of this RFC. You can read it's mentioned in comments above with the @ notation.

Well aware, I was the first respondent to this RFC! My question is this issue is now closed but I don't think branch support was added?

@pirafrank
Copy link

Well aware, I was the first respondent to this RFC! My question is this issue is now closed but I don't think branch support was added?

Sorry then, it's me missing the point.

I looked at the code and your PR hasn't been merged. I guess that's a make-or-break question only @sxyazi can answer.

@uncenter
Copy link
Contributor

My PR didn't add branch support, it added arbitrary Git forge/host support. Both would be great but at the minute we have neither.

@sxyazi
Copy link
Owner Author

sxyazi commented Aug 17, 2024

My question is this issue is now closed but I don't think branch support was added?

No, this RFC doesn't cover adding branch support; it only addresses the issue of # being recognized as a special character by the user's shell as the title says.

The RFC does mention that @ could be used as a future keyword for branches, but as I mentioned:

I'll try to implement the : part first in the next version and provide a gradual transition plan

I've only implemented the part with : for now - adding branch support introduces more complexity and it's not a high priority, I need to think it over. Currently, the package manager is just a minimum viable product.

Copy link

I'm going to lock this issue because it has been closed for 30 days. ⏳
This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RFC Request for Comments
Projects
None yet
4 participants