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: delete/uninstall packages #2181

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: delete/uninstall packages #2181

wants to merge 3 commits into from

Conversation

MrAsler
Copy link
Contributor

@MrAsler MrAsler commented Jan 9, 2025

Closes #2147

Details about the implementation

To add the delete command, I followed the same pattern that existed in the src/args.rs, src/main.rs and src/package/* directories.

I tried to follow the existing code style, and kept refactorings to a minimum - I created two helper methods - find_dep_in_package and deployed_directory. If there are any naming issues or you would like the code to be structured differently, feel free to say!

Follow up / Questions

Some code is still not deleted

As things are, this will not delete the package code that is present in Xdg::local() directory, as I don't understand why the state_dir() of a dependency is using an hash to identify the package (format!("{:x}", XxHash3_128::oneshot(self.remote().as_bytes()))) instead of just using the package name.
I'm assuming it is desired to also remove the content of this state_dir(), but I'll await for the answer 🕵️

Can the field Dependency.use_ be used the dependency ID?

The function Package::add(..) was detecting if a package existing with the following code:

if self.plugins.iter().any(|d| d.repo == dep.repo && d.child == dep.child) {
	bail!("Plugin `{name}` already exists in package.toml");
}

However, given that the objective is to see if a package already exists in package.toml and each package keeps the use value there, I'm under the impression that this can be simplified to using use_.
If this use field is internal and should not be used, should there be a way of giving an ID to a package, or should we keep using the parent and child names?

@sxyazi
Copy link
Owner

sxyazi commented Jan 10, 2025

Thanks for the PR, the code looks really neat!

I don't understand why the state_dir() of a dependency is using an hash to identify the package (format!("{:x}", XxHash3_128::oneshot(self.remote().as_bytes()))) instead of just using the package name

Yazi's package manager supports two types of packages: one is a plugin itself, like https://github.com/dedukun/relative-motions.yazi, and the other is a subpackage within a large monorepo, like https://github.com/yazi-rs/plugins/tree/main/jump-to-char.yazi.

For the second type, there is no package name, just the repo's name, so to flatten the structure, I encoded them all as hashes.

I'm assuming it is desired to also remove the content of this state_dir()

I think the current implementation is good enough for now. We can leave it as is and improve it in the future.

Can the field Dependency.use_ be used the dependency ID?

It can be used for now, but since Yazi plans to support other Git forges like GitLab and Bitbucket, which have different URL formats, we might end up with two different URLs pointing to the same package.

So using parent and child would be more reliable, as they are always parsed by Yazi first.

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.

Delete/uninstall plugin
2 participants