feat: add menu item install to pixi global
#2699
Conversation
|
|
||
|
|
||
| def test_menuinst_install(pixi: Path, tmp_pixi_workspace: Path, dummy_channel_1: str) -> None: | ||
| env = {"PIXI_HOME": str(tmp_pixi_workspace)} |
There was a problem hiding this comment.
Would be good to avoid cluttering the machine with things installed by our tests.
For Linux setting HOME should do. Let's check what's the best approach for macOS and Windows
There was a problem hiding this comment.
I'm not sure what you mean to say with this?
There was a problem hiding this comment.
We solved the confusion
| Serialize::serialize(self, toml_edit::ser::ValueSerializer::new()).unwrap() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Could you please add a test for a weird table name? E.g. with a dot inside?
No worries if it doesn't work, but we could open an issue for that
| } | ||
|
|
||
| /// Sets the platform for this environment | ||
| pub fn with_platform(mut self, platform: Option<Platform>) -> Self { |
There was a problem hiding this comment.
| pub fn with_platform(mut self, platform: Option<Platform>) -> Self { | |
| pub fn with_platform(mut self, platform: Platform) -> Self { |
You wouldn't call this method if it were None
| } | ||
|
|
||
| /// Sets the menu install flag for this environment | ||
| pub fn with_menu_install(mut self, menu_install: Option<bool>) -> Self { |
There was a problem hiding this comment.
| pub fn with_menu_install(mut self, menu_install: Option<bool>) -> Self { | |
| pub fn with_menu_install(mut self, menu_install: bool) -> Self { |
There was a problem hiding this comment.
I felt like the code flow was more readable if the stayed being an option as it does contain information being None vs Some(true) or Some(false)
| ), | ||
| ); | ||
| } | ||
| // Don't fail on menu install errors, menuinst is too unstable to break the whole process because of issue with it. |
There was a problem hiding this comment.
What makes menuinst so unstable that we wouldn't error on it?
There was a problem hiding this comment.
This might be something we want to test, but as it was a prototype state feature I didn't want to block users with it.
src/prefix.rs
Outdated
| let entry = entry.into_diagnostic()?; | ||
| let path = entry.path(); |
There was a problem hiding this comment.
Move those two line into one line
This is an initial implementation.
It will add a
menu-install = trueto your environments if it installs menu items.TODO: