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

UX and Test Improvements #1314

Merged
merged 50 commits into from
Dec 19, 2024
Merged

UX and Test Improvements #1314

merged 50 commits into from
Dec 19, 2024

Conversation

elcritch
Copy link
Contributor

@elcritch elcritch commented Dec 16, 2024

There's a few different UX improvements in this PR: progress spinner, colorized and prettified nimble deps, and an inverted deps format nimble deps --format=inverted.

Progress Spinner

Nimble can take a while to check git for package caches now among other things, which can lead to frustration not knowing if Nimble is working or hung on some repo.

This PR adds a spinner style progress indicator to make these waits less annoying. The progress is based on suppressed debug messages which gives a good cadence to the work being done.

The spinner is just a simple Working ⡿ style one, not a percentage one. It only shows when using noColor is not set.

image

Deps Tree

It's colorized and outputs a nice tree format borrowing from Cargo, NPM, etc.

image

Inverted Deps Tree

It's really helpful to print each dependency for a project and what requirements led to the resolved version.

image

@elcritch elcritch changed the title Show progress spinner on interactive terminals UX: Show progress spinner on interactive terminals Dec 16, 2024
@elcritch elcritch changed the title UX: Show progress spinner on interactive terminals UX Improvements Dec 16, 2024
@arnetheduck
Copy link

This is nice - nimble is way way way way too verbose and fills the screen with useless junk for basically anything it does - it should not be wasting vertical space on "I actually managed to perform the task you asked me to" as if this was a surprise, every time.

The spinner is just a simple

instead of "working", it could show what it's doing (ie Scanning xyz or Cloning ...)

@elcritch
Copy link
Contributor Author

instead of "working", it could show what it's doing (ie Scanning xyz or Cloning ...)

I agree. However, there's limited info on what it's doing at the display proc level.

Maybe I'll give it another take tomorrow. Maybe just some line matching as 90% of the debugging output comes from only a handful of tasks.

@arnetheduck
Copy link

some line matching

this sounds like a maintenance nightmare though - maybe better leave it as is until a more deliberate source of information can be developed

@elcritch
Copy link
Contributor Author

elcritch commented Dec 16, 2024

this sounds like a maintenance nightmare though - maybe better leave it as is until a more deliberate source of information can be developed

It could be. Though it shouldn't result in anything breaking just the message going to the default "Working".

I added it as I was curious and it doesn't seem too bad. Does make it nicer to know it's "Scanning" (printPkgInfo) or "Updating" (git ls-remote).

Though the idea is nice, but probably best to revert it. It'd be better and probably not too hard to pipe the info in via the doCmd pieces.

@elcritch
Copy link
Contributor Author

This is nice - nimble is way way way way too verbose and fills the screen with useless junk for basically anything it does - it should not be wasting vertical space on "I actually managed to perform the task you asked me to" as if this was a surprise, every time.

Given that the resolver uses SAT now and there's a deps command it could be a good chance to remove this sorta stuff:

  Success:  threading installed successfully.
  Verifying dependencies for sigils@0.8.5
     Info:  Dependency on variant@>= 0.2.12 already satisfied
  Verifying dependencies for variant@0.3.1
     Info:  Dependency on threading@#master already satisfied
  Verifying dependencies for threading@0.2.0
     Info:  Dependency on stack_strings@any version already satisfied
  Verifying dependencies for stack_strings@1.1.4
     Info:  Dependency on patty@>= 0.3.4 already satisfied
  Verifying dependencies for patty@0.3.5
 Installing sigils@0.8.5

@elcritch elcritch changed the title UX Improvements UX and Test Improvements Dec 18, 2024
@elcritch
Copy link
Contributor Author

@jmgomez this is ready. Not sure who's reviewing these things nowadays so figured I'd ping you.

@jmgomez
Copy link
Collaborator

jmgomez commented Dec 19, 2024

Looks good to me. But @Araq is the one approving for the time being :)

@Araq Araq merged commit 5a5e2a4 into nim-lang:master Dec 19, 2024
11 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.

4 participants