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.

Sign up for GitHub

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

Jump to bottom

search.cc: improve UX for nix search #2158

Merged
merged 1 commit into from Jul 2, 2018
Merged

search.cc: improve UX for nix search #2158

merged 1 commit into from Jul 2, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented May 12, 2018 •
edited

As proposed in #1634 the nix search command could use some
improvements. Initially 0413aeb added
some basic sorting behavior using std::map, a next step would be an
improvement of the output.

This patch includes the following changes:

  • Use $PAGER for outputs with RunPager from shared.hh:
    The same behavior is defined for nix-env --query, furthermore it
    makes searching huge results way easier.

  • Simplified result blocks:
    The new output is heavily inspired by the output from nox, the first
    line shows the attribute path and the derivaiton name
    (attribute path (derivation name)) and the description in the second
    line.

@Ma27
Copy link
Member Author

Ma27 commented May 13, 2018

Please keep in mind that this is quite opinionated as I'd like to improve the UX for nix search.
In case anybody disagrees with some of the changes (or has better ideas) I'm happy to discuss / try it out :)

@edolstra
Copy link
Member

👍 on using the pager (in fact it should be enabled for all nix commands that produce output, but it might interact badly with the progress indicator).

However I'm not sure if hiding most of the package information is a good idea. I also don't know if it's actually a UX improvement to make the user guess what the meaning of the output columns is.

@Ma27
Copy link
Member Author

Ma27 commented May 14, 2018

First of all, thanks a lot for your feedback!

However I'm not sure if hiding most of the package information is a good idea. I also don't know if it's actually a UX improvement to make the user guess what the meaning of the output columns is.

you might be right. The main reason why I filed this patch was because I think that the current nix search interface is way too bloated and too hard to read. Although it's just a proposal, what do you think of the nix search --verbose output (which is inspired by nox <searchterm>)?

In the end we could simply drop the --verbose argument for nix search, however it'd be extremely cool to get some feedback for the new output I built from your for the side as first %)

@Ma27
Copy link
Member Author

Ma27 commented May 24, 2018

@edolstra before I continue working on this patch, are you happy with the verbose output?
I could remove the --verbose behavior in nix search for now (we can talk about this in a separate ticket, right?), but I'd really love to improve the output of nix search as I consider the current output as too hard to read.

Furthermore I'd be happy to discuss about the verbose output (which I'd really love to see improved) %)

@jcrben
Copy link

jcrben commented May 25, 2018 •
edited

Agreed that the current output is far too verbose. I tend to avoid using nix search (and use nix-env instead) for that reason. Seems like we @Ma27 did you consider including a screenshot of what your new output looks like?

@Ma27
Copy link
Member Author

Ma27 commented May 25, 2018

@jcrben

Screenshot for nix search on imgur

@edolstra
Copy link
Member

That looks pretty okay to me, except that I'd prefer swapping the attribute name and package/version name, since we're de-emphasizing the latter. (I.e. it should be * nixpkgs.sblim-sfcc (sblim-sfcc-2.2.9).)

@Ma27
Copy link
Member Author

Ma27 commented May 25, 2018 •
edited

@edolstra you're right, fixed it.
The last issue (I guess) before we could merge this is whether or not to use --verbose: do you think it would make sense to provide a non-verbose version (with at least the attribute path) or should we drop this entirely and show the view I posted above by default?

@Ma27
Copy link
Member Author

Ma27 commented Jun 19, 2018

@edolstra any chance to get this merged or is something else missing?

@edolstra
Copy link
Member

The only remaining problem is that it overloads the meaning of --verbose. That flag has always only affected log output (i.e. stderr), now it also has an effect on the primary output (stdout). So nix search -v will not only show more details about packages, but it may also print more log messages. Or, if we ever added a nix.conf option to set the default verbosity level, it would also affect the output of nix search.

Does anybody have a suggestion for an alternative flag name?

@Ma27
Copy link
Member Author

Ma27 commented Jun 20, 2018

The only remaining problem is that it overloads the meaning of --verbose. That flag has always only affected log output (i.e. stderr), now it also has an effect on the primary output (stdout). So nix search -v will not only show more details about packages, but it may also print more log messages. Or, if we ever added a nix.conf option to set the default verbosity level, it would also affect the output of nix search.

You're absolutely right, I've just seen that Nix supports --verbose by default and used it, but I absolutely agree with you, it doesn't make any sense to use a single flag for extended logging and enhanced outputs.

Does anybody have a suggestion for an alternative flag name?

Something like --detailed?

But as you stated previously do you think we should keep such a flag or would it be better to print the enhanced output (as shown in the screenshot above) by default? When I filed this patch I thought that it might be more convenient to have a minimalistic output by default (which is why I (ab)used --verbose), but now I'm not sure if we actually need this 😅

@Ma27
search.cc: improve UX for nix search
6b74fda 
As proposed in NixOS#1634 the `nix search` command could use some
improvements. Initially 0413aeb added
some basic sorting behavior using `std::map`, a next step would be an
improvement of the output.

This patch includes the following changes:

* Use `$PAGER` for outputs with `RunPager` from `shared.hh`:
  The same behavior is defined for `nix-env --query`, furthermore it
  makes searching huge results way easier.

* Simplified result blocks:
  The new output is heavily inspired by the output from `nox`, the first
  line shows the attribute path and the derivaiton name
  (`attribute path (derivation name)`) and the description in the second
  line.
@Ma27
Copy link
Member Author

Ma27 commented Jul 2, 2018

@edolstra I dropped the --verbose usage, I guess that two lines per search result are fairly reasonable, right?

@edolstra edolstra merged commit dd98683 into NixOS:master Jul 2, 2018
@edolstra
Copy link
Member

edolstra commented Jul 2, 2018

Thanks!

@Ma27 Ma27 deleted the improve-search-ux branch July 2, 2018 10:05
@Ma27 Ma27 mentioned this pull request Jul 2, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ma27 @edolstra @jcrben