-
Notifications
You must be signed in to change notification settings - Fork 171
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: improve sync reporting #2202
base: feature/pixi-global
Are you sure you want to change the base?
feat: improve sync reporting #2202
Conversation
#[derive(Debug, Default)] | ||
pub(crate) struct StateChanges { | ||
changes: Vec<StateChange>, | ||
has_updated: bool, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the interesting bit. By returning this struct we can pick and choose what to report. For example, we never want to report changes done by reverting.
src/global/common.rs
Outdated
impl fmt::Display for StateChange { | ||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sure the reporting is unified across commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be nice if we could make a formatter that expects an env_name
and then formats the change a bit more informative. e.g. this is the output of installing python:
❯ pixi g i python
✔ Added environment 'python'.
✔ Added package 'python' and installed it with version: '3.12.7' in environment: 'python' .
✔ Exposed '2to3' binary from environment 'python'.
✔ Exposed '2to3-3.12' binary from environment 'python'.
✔ Exposed 'idle3' binary from environment 'python'.
✔ Exposed 'idle3.12' binary from environment 'python'.
✔ Exposed 'pydoc' binary from environment 'python'.
✔ Exposed 'pydoc3' binary from environment 'python'.
✔ Exposed 'pydoc3.12' binary from environment 'python'.
✔ Exposed 'python' binary from environment 'python'.
✔ Exposed 'python3' binary from environment 'python'.
✔ Exposed 'python3-config' binary from environment 'python'.
✔ Exposed 'python3.1' binary from environment 'python'.
✔ Exposed 'python3.12' binary from environment 'python'.
✔ Exposed 'python3.12-config' binary from environment 'python'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ChatGPT came with this idea:
✔ Added environment 'python' with package 'python' (version: 3.12.7).
✔ Exposed binaries from environment 'python':
- 2to3, 2to3-3.12
- idle3, idle3.12
- pydoc, pydoc3, pydoc3.12
- python, python3, python3.12
- python3-config, python3.12-config
src/global/common.rs
Outdated
fn prune(&mut self) { | ||
// Remove changes if the environment is removed afterwards | ||
let mut pruned_changes: Vec<StateChange> = Vec::new(); | ||
for change in &self.changes { | ||
match change { | ||
StateChange::RemovedEnvironment(env) => { | ||
pruned_changes.retain(|change| match change { | ||
StateChange::RemovedEnvironment(_) => true, | ||
c => c.env() != env, | ||
}); | ||
} | ||
_ => { | ||
pruned_changes.push(change.clone()); | ||
} | ||
} | ||
} | ||
self.changes = pruned_changes; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, prune
only makes sure that we don't report changes on an environment we later remove. This can be extended in the future.
For example, when implementing pixi global remove
we could make sure that RemovedPackage
and AddedPackage
cancel each other out.
This moves from functions printing to
stderr
to functions returningStateChange
orStateChanges
.The reporting is then done higher up.