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

ya pack pretty prints plugin config #1020

Closed

Conversation

mikavilpas
Copy link
Contributor

@mikavilpas mikavilpas commented May 9, 2024

Example of a pretty printed configuration:

[[plugin.deps]]
use = "DreamMaoMao/keyjump.yazi"
commit = "06383de"

[[plugin.deps]]
use = "Rolv-Apneseth/starship.yazi"
commit = "6197e4c"

[[plugin.deps]]
use = "Reledia/glow.yazi"
commit = "536185a"

[flavor]
deps = []

This is how it would look if not pretty printed:

[plugin]
deps = [{ use = "DreamMaoMao/keyjump.yazi", commit = "06383de" }, { use = "Rolv-Apneseth/starship.yazi", commit = "6197e4c" }, { use = "Reledia/glow.yazi", commit = "536185a" }]

[flavor]
deps = []

Error reporting (in case the configuration is invalid) now looks like this:

# in this example I removed one "use" field from the config
$ w cargo run --bin ya -- pack -a Reledia/glow.yazi
[Running: cargo run --bin ya -- pack -a Reledia/glow.yazi]
   Compiling yazi-boot v0.2.5 (/Users/mikavilpas/git/yazi/yazi-boot)
   Compiling yazi-cli v0.2.5 (/Users/mikavilpas/git/yazi/yazi-cli)
   Compiling yazi-dds v0.2.5 (/Users/mikavilpas/git/yazi/yazi-dds)
    Finished dev [unoptimized + debuginfo] target(s) in 6.18s
     Running `/Users/mikavilpas/git/yazi/target/debug/ya pack -a Reledia/glow.yazi`
Error: Failed to parse package.toml

Caused by:
    TOML parse error at line 9, column 1
      |
    9 | [[plugin.deps]]
      | ^^^^^^^^^^^^^^^
    missing field `use`

Note: this change rebuilds the configuration parsing by switching the toml_edit crate to toml. This means the plugin system is now declarative instead of procedural.

I find this significantly simplifies the implementation, but let me know what you think!

This allows using plugins that end in ".yazi"
This way the user can easily diagnose the situation without knowing the
internals of the yazi plugin system.
A pretty printed configuration file makes it easier to see the
differences between two versions of the configuration file.
@mikavilpas mikavilpas force-pushed the ya-pack-pretty-prints-plugin-config branch from ef3ef24 to 58b6e75 Compare May 9, 2024 16:42
@mikavilpas
Copy link
Contributor Author

Oops, looks like plugins now get added as duplicates.. Let me fix that!

@mikavilpas
Copy link
Contributor Author

Ok, duplicate plugin addition has now been fixed!

@sxyazi
Copy link
Owner

sxyazi commented May 9, 2024

Hi, thanks for the PR!

Using toml_edit instead of toml is expected behavior because it better respects the user's configuration style, as package.toml is essentially the user's configuration file. Reformatting it could disrupt this, potentially conflicting with the user's editor's TOML formatting plugin. So my thinking is:

  • For users who always manage dependencies via CLI and rarely open this file manually, messy formatting might not be an issue.
  • For users who need to modify this file, their editor plugins will automatically format it, or manually, and only newly added dependency lines look messy, which won't affect existing content.

Therefore, I'd prefer to keep the current behavior. Curious your thoughts!

@mikavilpas
Copy link
Contributor Author

Oh, I see. I think this is about the difference between a recipe file and a lock file.

A recipe file:

  • specifies the dependent packages on a high level
  • (in my experience) typically edited manually
  • examples:
    • Cargo.toml (rust)
    • package.json (npm)
    • Gemfile (Ruby)
    • plugin/yazi.lua (Neovim with lazy.nvim)

A lock file:

  • specifies the exact versions of the dependent packages
    • version
    • git branch
    • registry URL
    • cryptographic hash
    • (other metadata)
  • (in my experience) typically generated and formatted automatically
  • examples:
    • Cargo.lock (rust)
    • package-lock.json (npm)
    • Gemfile.lock (Ruby)
    • lazy-lock.json (lazy.nvim)

I think what I have done here is treat package.toml as a lock file. I'm not sure if the difference between these two is clear in this case.

I think it comes down to preference, and what you think is best should be followed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants