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

Prettier UI #260

Closed
wants to merge 16 commits into from
Closed

Prettier UI #260

wants to merge 16 commits into from

Conversation

passcod
Copy link
Member

@passcod passcod commented Jul 30, 2022

Closes #156

  • Introduces a new UI layer and disables log output by default
  • If the terminal is detected to be non-interactive, disables the UI and re-enables log output, and also assumes --no-confirm
  • Tries to do reflink copying when supported
  • Renames --no-symlinks to --no-versioned (keeps the old name as alias for now)
  • Switches how --no-versioned behaves to be in line with the option help text and align with expectations
  • For now, source builds are done in series... looking into building in pty

Peek 2022-07-30 15-47

Peek 2022-07-30 15-49

Peek 2022-07-30 15-45

Peek 2022-07-30 15-50


When the terminal is not interactive, it defaults to no-confirm and switches to log view:

Peek 2022-07-30 15-53

Peek 2022-07-30 15-52

@passcod passcod marked this pull request as draft July 30, 2022 04:50
@passcod passcod force-pushed the ux branch 2 times, most recently from e175a18 to 14bbc9d Compare July 30, 2022 05:21
@passcod passcod force-pushed the ux branch 2 times, most recently from 55070bf to 8195e0e Compare July 30, 2022 05:33
src/bins.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@passcod passcod requested a review from NobodyXu July 30, 2022 11:07
src/bins.rs Outdated Show resolved Hide resolved
src/bins.rs Show resolved Hide resolved
Ok(())
}

fn install_copy(src: &Path, dst: &Path) -> Result<(), BinstallError> {
Copy link
Member

Choose a reason for hiding this comment

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

Can we make install_copy atomic by first creating a NamedTempFile?

);

#[cfg(unix)]
{
Copy link
Member

Choose a reason for hiding this comment

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

It's actually possible to also make symlink atomic.

Check the original helpers::atomic_symlink for implementation.

src/bins.rs Outdated Show resolved Hide resolved
@NobodyXu
Copy link
Member

NobodyXu commented Aug 8, 2022

There are so many merge conflicts on this PR...
I am going to pause writing up new PRs so that @passcod you can take your time and finish this PR.

@passcod
Copy link
Member Author

passcod commented Aug 20, 2022

Going to close this and redo it later

@passcod passcod closed this Aug 20, 2022
@NobodyXu
Copy link
Member

Going to close this and redo it later

It's probably better to split it into multiple small PRs as that would make merging much easier and less likely to have conflicts.

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.

Possible UI / UX improvements?
2 participants