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

feat: support assets installation for the ya pack subcommand #1973

Merged
merged 3 commits into from
Dec 1, 2024

Conversation

zooeywm
Copy link
Contributor

@zooeywm zooeywm commented Nov 30, 2024

feature for #1947 , haven tested based on my sudo.yazi fork

❯ ./ya pack --add=zooeywm/sudo

  Upgrading package `sudo.yazi`

remote: Enumerating objects: 5, done.
remote: Counting objects: 100% (5/5), done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 3 (delta 2), reused 3 (delta 2), pack-reused 0 (from 0)
Unpacking objects: 100% (3/3), 544 bytes | 544.00 KiB/s, done.
From https://github.com/zooeywm/sudo.yazi
   4f446d3..8a3c803  main       -> origin/main
Previous HEAD position was 4f446d3 chore(ya): `ya pack` -a support assets folder contains miscellaneous files
HEAD is now at 8a3c803 chore(yazi): https://github.com/sxyazi/yazi/pull/1966

❯ tree ~/.config/yazi/plugins/sudo.yazi
/home/zooeywm/.config/yazi/plugins/sudo.yazi
├── assets
│   └── fs.nu
├── DO_NOT_MODIFY_ANYTHING_IN_THIS_DIRECTORY
├── init.lua
├── LICENSE
└── README.md

@boydaihungst
Copy link
Contributor

Is there an option to use a different folder name instead of assets? For example: ya pack -a --assets=folder_x.
Some plugin repositories already have an assets folder, which is used for image previews.

@sxyazi sxyazi mentioned this pull request Nov 30, 2024
3 tasks
@sxyazi
Copy link
Owner

sxyazi commented Nov 30, 2024

@boydaihungst Specifying the assets directory name when adding a plugin means you'll still need to specify it when uninstalling, otherwise Yazi won't know if the user has used a custom assets name, which can mess with automation a bit.

Also, this doesn't seem to affect existing plugins - it's just that directories that weren't copied before will now be copied during installation.

On another note, it's not recommended to put preview images alongside the code because they're large binaries, which can slow down the cloning process when installing plugins.

The recommended approach is to drag the image into the markdown file to upload it, and GitHub will automatically create a permanent link for it, like this https://github.com/yazi-rs/plugins/blob/a53d5440481f0f9a2160ded47d343bd22ffbc1fb/chmod.yazi/README.md?plain=1#L5 (not an image but a video though).

@boydaihungst
Copy link
Contributor

boydaihungst commented Nov 30, 2024

@sxyazi Alright. To makes it less complicated, I will rename my plugins's assets folder to something else. 👍

@sxyazi
Copy link
Owner

sxyazi commented Nov 30, 2024

Thanks for the PR @zooeywm!

I tested it and found that there seems to be an issue with file status synchronization, where files in the (local plugin's) assets folder are never deleted after running ya pack -i again, even though the remote plugin's assets directory no longer contains those files.

@zooeywm
Copy link
Contributor Author

zooeywm commented Nov 30, 2024

Hi @sxyazi , I've just tried to solve the synchronization between config files status and local state files in commit 5ef2e81.

By write rev to the TRACK file, check if the recorded rev is the same as new rev, if not, the DiffAction with be make with Git module and processed by deploy(). I have also use the remove_dir_clean() to remove empty folders.

Thanks for review and hope that works!

@sxyazi sxyazi changed the title feat: support ya for miscellaneous files in folder assets feat: support assets installation for the ya pack subcommand Dec 1, 2024
Copy link
Owner

@sxyazi sxyazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sxyazi sxyazi merged commit 91665b7 into sxyazi:main Dec 1, 2024
6 checks passed
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.

3 participants