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

Mixed comments and snippets in search mode #137

Closed
SamuelDSR opened this issue Oct 24, 2019 · 18 comments · Fixed by #138
Closed

Mixed comments and snippets in search mode #137

SamuelDSR opened this issue Oct 24, 2019 · 18 comments · Fixed by #138

Comments

@SamuelDSR
Copy link

Issue Description

Environment

  • System: Ubuntu18.04
  • Installation method: linuxbrew

When search a snippet, the comments and commands of all candidate snippet are interleved in a random way.
Please see snapshot below :)

image

@welcome
Copy link

welcome bot commented Oct 24, 2019

Thanks for opening your first issue here! In case you're facing a bug, please update navi to the latest version first. Maybe the bug is already solved! :)

@wallace11
Copy link

Yeah I agree too that this is very confusing. We already have the preview window so maybe we can have the list show only desc / only command and the preview show the other one (should be configurable imho).

@denisidoro
Copy link
Owner

Actually, it's not displayed randomly. It shows "best" matches in descending order, according to fzf.

I'm not sure about displaying only comments or only snippets because many people (myself included) search by both, depending on the use. An option could be added to filter these results, but that would limit the use of the cheatsheets and I believe only a few people would really use it.

Any other ideas?

@denisidoro
Copy link
Owner

denisidoro commented Oct 24, 2019

Ah, one important info: there's no way to hide the snippets and search for them, for example. fzf requires it to be visible printed to be searchable

@denisidoro
Copy link
Owner

One can use the hidden ANSI code to hide some content, but that won't work on most terminal emulators

@wallace11
Copy link

Yeah, you have a point there
Maybe it's possible to put the command/comment on the same line with muted color next to category.
It's almost certainly going to overflow but fzf auto-calculates that and we have the preview to compensate.

@denisidoro
Copy link
Owner

Yeah, I'm thinking on something like

change branch  [git]  git checkout
get logs       [git]  git logs
reset          [git]  git reset

@denisidoro
Copy link
Owner

denisidoro commented Oct 24, 2019

Some quick experiments:

Capture d’écran 2019-10-24 à 12 16 41
Capture d’écran 2019-10-24 à 12 17 27
Capture d’écran 2019-10-24 à 12 17 57
Capture d’écran 2019-10-24 à 12 19 49

@denisidoro
Copy link
Owner

denisidoro commented Oct 24, 2019

I believe the width of a given column should be limited otherwise all the other content will become invisible. I'm not sure if we should use magic numbers or make if dependent on the terminal width.

@denisidoro
Copy link
Owner

denisidoro commented Oct 24, 2019

WDYT?

Capture d’écran 2019-10-24 à 12 29 51

@denisidoro
Copy link
Owner

This way it will become impossible to read the whole comment if it's too big, though

@denisidoro
Copy link
Owner

The preview could always contain 2 lines: one for the comment and another one for the snippet :)

@denisidoro
Copy link
Owner

Capture d’écran 2019-10-24 à 13 10 32

@wallace11
Copy link

Last one looks superb.
If you provided the full echo command I can paste here more examples of how it looks.

@denisidoro
Copy link
Owner

I'm about to open a PR so you can test it using another branch 👍

@denisidoro
Copy link
Owner

denisidoro commented Oct 24, 2019

@SamuelDSR and @wallace11: could you try the table/refactor branch and give your feedback?

@SamuelDSR
Copy link
Author

@SamuelDSR and @wallace11: could you try the table/refactor branch and give your feedback?

The branch table/refactor works perfectly in search mode under both mac iterm2/ ubuntu bash [tested]. Thanks very much. @denisidoro

Capture d’écran 2019-10-28 à 11 25 13

@denisidoro
Copy link
Owner

Cool! The branch introduces new options to customize which columns are shown and so on. I'll probably merge it this week

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 a pull request may close this issue.

3 participants