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

[MVP][WIP] less like pager #6984

Merged
merged 68 commits into from
Dec 1, 2022
Merged

[MVP][WIP] less like pager #6984

merged 68 commits into from
Dec 1, 2022

Conversation

zhiburt
Copy link
Contributor

@zhiburt zhiburt commented Nov 2, 2022

Run it as explore.

example

ls | explore

Configuration points in config.nu file.

  # A 'explore' utility config
   explore_config: {
     highlight: { bg: 'yellow', fg: 'black' }
     status_bar: { bg: '#C4C9C6', fg: '#1D1F21' }
     command_bar: { fg: '#C4C9C6' }
     split_line: '#404040'
     cursor: true
     # selected_column: 'blue'
     # selected_row: { fg: 'yellow', bg: '#C1C2A3' }
     # selected_cell: { fg: 'white', bg: '#777777' }
     # line_shift: false,
     # line_index: false,
     # line_head_top: false,
     # line_head_bottom: false,
   }

You can start without a pipeline and type explore and it'll give you a few tips.
image

If you type :help you an see the help screen with some information on what tui keybindings are available.
image

From the :help screen you can now hit i and that puts you in cursor aka inspection mode and you can move the cursor left right up down and it you put it on an area such as [table 5 rows] and hit the enter key, you'll see something like this, which shows all the : commands. If you hit esc it will take you to the previous screen.
image

If you then type :try you'll get this type of window where you can type in the top portion and see results in the bottom.
image

The :nu command is interesting because you can type pipelines like :nu ls | sort-by type size or another pipeline of your choosing such as :nu sys and that will show the table that looks like this, which we're calling "table mode".
image

If you hit the t key it will now transpose the view to look like this.
image

In table mode or transposed table mode you can use the i key to inspect any collapsed field like {record 8 fields}, [table 16 rows], [list x], etc.

One of the original benefits was that when you're in a view that has a lot of columns, explore gives you the ability to scroll left, right, up, and down.

explore is also smart enough to know when you're in table mode versus preview mode. If you do open Cargo.toml | explore you get this.
image

If you type open --raw Cargo.toml | explore you get this where you can scroll left, right, up, down. This is called preview mode.
image

When you're in table mode, you can also type :preview. So, with open --raw Cargo.toml | explore, if you type :preview, it will look like this.
image

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt zhiburt changed the title [MVP][WIP] Less like pager [MVP][WIP] less like pager Nov 2, 2022
@fdncred
Copy link
Collaborator

fdncred commented Nov 3, 2022

whoa! that's so cool. trying to figure how to get out of it now though. LOL - figured it out ctrl+d. totally love this and would love to see it grow. this is exactly a mvp version of what I meant with csvlens. great job @zhiburt!!!

@webbedspace
Copy link
Contributor

I don't like the pun name, for these reasons:

  • Too difficult to "get" if you don't already know what the two components are (otherwise, looks like a typo of tables).
  • Personally don't like the name of table due to the word "table" describing too many things in Nushell (Consider renaming the table command to something more accurate #6772) and this doubles down on it.
  • less itself is starting to fall out of favour in place of third-party alternatives like bat, so the name's resonance (in terms of describing the concept of pagers itself) is fading.
  • Pun names are typically the domain of third-party programs trying to have memorable branding (e.g. "Ag, the Silver Searcher") and not internal commands of a utility. Typical Nushell command naming is usually a more direct verb instead of a POSIX name (find instead of grep, fetch instead of curl etc.) with exceptions only given to directory navigation (mkdir etc.)

My proposal is renaming it to something like pager or scroll.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@fdncred
Copy link
Collaborator

fdncred commented Nov 3, 2022

@zhiburt This is my torture test for all things related to tables. :)
opening this file reveals some interesting artifacts.
em.csv

i've learned to expect that nearly no software can draw this correctly. however, a tui has potential if it could calculate the column widths of the total file and pad it, although collecting for measurement isn't ideal.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 3, 2022

I did a few improvements here and there and I think it's more or less in a good state.

A few questions;

  1. What should it return to the pipe? (PipelineState)
    I think nothing?

  2. How it shall work with basic types like (Boolean, Int etc.) and ExternalStream.

  3. I am thinking to add PG_UP, PG_DOWN keys to scroll pages. But is there more useful commands?
    PS: It could be left as a GOOD FIRST ISSUE in case it will be merged.

opening this file reveals some interesting artifacts.

I've run through some part of it, and seems OK.
Could you show it? (I am sure there will be ones)

I don't like the pun name, for these reasons:

Good points; @webbedspace
I've renamed it.

@fdncred
Copy link
Collaborator

fdncred commented Nov 3, 2022

@zhiburt I think PGUP and PGDOWN are good and also letting esc, ctrl-c along with ctrl-d to exit it.

Notice the artifacts of lines on the right side and sometimes text not erasing as well as the columns jumping around. not sure if there's anything you can do about these things but it's what i was talking about.
scroll

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 3, 2022

Notice the artifacts of lines on the right side and sometimes text not erasing as well as the columns jumping around. not sure if there's anything you can do about these things but it's what i was talking about.

Interesting....

Could you check whether it was fixed?

@fdncred
Copy link
Collaborator

fdncred commented Nov 4, 2022

Could you check whether it was fixed?

Not fixed. It still exhibits the same artifacts. I tried in Windows Terminal and in Tabby.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 4, 2022

Could you check whether it was fixed?

Not fixed. It still exhibits the same artifacts. I tried in Windows Terminal and in Tabby.

that's interesting.


I've spend some time to do a recursive logic to get inside inner values.
(yet not completed).

But wanted to know whether it's worth to continue.

image

The idea is if the value is actually a record/list we can inspect it further in a pop up instead of having { record .. }, [ list ... ] message.

PS: Personally I kind of feel it's more logic than a simple pager like less/more shall have. So it may be better splitted as a completely different binary.

@fdncred
Copy link
Collaborator

fdncred commented Nov 5, 2022

Oh wow! Ya, that's totally cool. In think being able to inspect things is a good add.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 5, 2022

I hope I made it work.

image

Press i to get in a 'cursor' mode
Use 'Arrow keys' to select a cell you want
Press enter, and you must get in inner view.
To exit it press ESC

PS: Haven't yet looked at your artifact issue.

@fdncred
Copy link
Collaborator

fdncred commented Nov 5, 2022

Nice, this is cool. So, if the list is a table it shows like ls -al | scroll shows but if it has nested collections, it allows you to go into them with i. Is that how it works?

Also, it needs some highlighting or something to show you where your cursor is or what row is selected. For instance, I did $nu | scroll and couldn't figure how to get i to work nor could I see if I was in cursor mode.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 6, 2022

Not sure if you ran it;
Here's a gif.

I've changed a an inner view.

download (4)

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@fdncred
Copy link
Collaborator

fdncred commented Nov 7, 2022

Not sure if you ran it;

I ran it but it looks different/better now.
I did $nu.scope.commands | scroll hit i and went down to ansi gradient then scrolled over to signature and hit enter and got this screen. Is there a way to scroll over to the right where it says custom_...? I couldn't get it to scroll.
image

Ok, here's a "fancy" suggestion that would be cool to do at some point. While in this screen.
image

It would be cool to hit / and be able to search in whichever column you're in.

Another suggestion, It would be nice to have something other than a blinking pipe to show your current selection, like highlight the entire row in reverse video or choose some background color for the whole row.

This is really starting to get me excited for this feature. I'm loving it! good job!!!

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 8, 2022

I did $nu.scope.commands | scroll hit i and went down to ansi gradient then scrolled over to signature and hit enter and got this screen. Is there a way to scroll over to the right where it says custom_...? I couldn't get it to scroll.

So you need to press ESC once to change mode back to 'view' then you'll must be able to do so.

It would be cool to hit / and be able to search in whichever column you're in.

Added some support of it, though not 'column based'.

  1. Press / to enter the search hit Enter next;
  2. You can iterate over results by pressing n

Another suggestion, It would be nice to have something other than a blinking pipe to show your current selection, like highlight the entire row in reverse video or choose some background color for the whole row.

Arguable thing, but surely being able to make it more customarily could be great. (like whether you you use cursor or selection etc.)

PS: I think you search in column is good idea, I also can picture different kind of sorts could be beneficial too (like search by the whole structure so if we found something somewhere deep in the value tree we would be able to get in there. right from search)

@fdncred
Copy link
Collaborator

fdncred commented Nov 8, 2022

nice - looks good! it's really fun to play with.

…tui-b

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 29, 2022

I'm willing to try it.

Must be done

Let's go with that then, please.

Renamed to explore

@fdncred
Copy link
Collaborator

fdncred commented Nov 29, 2022

Thanks @zhiburt. I tested with open -r Cargo.toml | explore and it went into "preview" mode. Then I did open Cargo.toml | explore and it went into table mode. I think this is what JT was looking for.

One quick question, Is there a way to debug the keybindings? I can't get pageup and pagedown to work in Windows in preview mode.

Here's another little bug?

  1. launch by typing explore
  2. type :nu open -r cargo. Lock
  3. it's in table mode vs preview mode (had to do i enter to see the file). this should probably auto detect too if possible. pageup/pagedown work fine here.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 29, 2022

One quick question, Is there a way to debug the keybindings?

I don't think so but how it would look?

I can't get pageup and pagedown to work in Windows in preview mode.

There was no such keybindings, but I've added it.

it's in table mode vs preview mode (had to do i enter to see the file). this should probably auto detect too if possible.

Made it so.

@fdncred
Copy link
Collaborator

fdncred commented Nov 30, 2022

I don't think so but how it would look?

As long as the keybindings work, there's no need for it. I was just wondering if there was something like keybindings listen for the tui. We don't really need it though since it wasn't a conflict that caused pageup/pagedown not to work. Thanks for adding that!

@fdncred
Copy link
Collaborator

fdncred commented Nov 30, 2022

Playing with it again this morning. Found a small bug.

  1. open -r cargo.toml | explore
  2. pageup pagedown (yay it works!)
  3. :try
  4. ls | sort size <-- boom, red error
  5. figured out that i should've typed sort-by. i try sort-by and it works, but the red error is still there. that is the bug.

Logging these ideas here (not required for this PR but will want to do eventually)

  1. in :try mode, it would be great to highlight the rectangle that has the focus. it's hard to tell when you can hit i versus type i
  2. mouse wheel scrolling would be nice

crates/nu-cli/src/util.rs Outdated Show resolved Hide resolved
@kubouch
Copy link
Contributor

kubouch commented Nov 30, 2022

We all agreed to land this, just a few small things. Could you write up what user-facing changes you introduced? Seems like there is at least the explore command added. Also there is a bunch of other commands available inside the nu-explore crate, this would be great to document as well (seems like they're not added to the default Nushell context). Some small user guide (like that you can type :help) that we could put into release notes would be useful as well.

Thanks for the hard work, it really feels amazing and is a huge step forward in Nushell's interface!

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@zhiburt
Copy link
Contributor Author

zhiburt commented Nov 30, 2022

Playing with it again this morning. Found a small bug.

  1. open -r cargo.toml | explore
  2. pageup pagedown (yay it works!)
  3. :try
  4. ls | sort size <-- boom, red error
  5. figured out that i should've typed sort-by. i try sort-by and it works, but the red error is still there. that is the bug.

Fixed.

It's actually was not a bug but a logic :)
I mean when everything go well with no errors we have nothing to show, so we don't update that bar. (yes... even if it has a error).

  1. in :try mode, it would be great to highlight the rectangle that has the focus. it's hard to tell when you can hit i versus type i

Not sure what is the best way.
But addressed it.

  1. mouse wheel scrolling would be nice

Not sure whether it's good but I've made is so. (To be candor it's a bit laggy?)

@zhiburt
Copy link
Contributor Author

zhiburt commented Dec 1, 2022

We all agreed to land this, just a few small things. Could you write up what user-facing changes you introduced? Seems like there is at least the explore command added. Also there is a bunch of other commands available inside the nu-explore crate, this would be great to document as well (seems like they're not added to the default Nushell context). Some small user guide (like that you can type :help) that we could put into release notes would be useful as well.

Only explore command was added.
No other changes were supposed to be made.

Yes I guess some sort of manual would be beneficial.

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
…tui-b

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@kubouch
Copy link
Contributor

kubouch commented Dec 1, 2022

We all agreed to land this, just a few small things. Could you write up what user-facing changes you introduced? Seems like there is at least the explore command added. Also there is a bunch of other commands available inside the nu-explore crate, this would be great to document as well (seems like they're not added to the default Nushell context). Some small user guide (like that you can type :help) that we could put into release notes would be useful as well.

Only explore command was added. No other changes were supposed to be made.

Yes I guess some sort of manual would be beneficial.

I mean, could you put these into the PR description (see the PR template)? It makes it easier for us when compiling the release notes and if somebody wants to look up the PR later for some reason.

@kubouch
Copy link
Contributor

kubouch commented Dec 1, 2022

It also introduced some new config settings, these are also user-facing changes.

@fdncred
Copy link
Collaborator

fdncred commented Dec 1, 2022

I updated the OP's issue text a bit, just to change the name to explore and add the config section.

@fdncred
Copy link
Collaborator

fdncred commented Dec 1, 2022

@zhiburt I found another glitch. Let me know if this is supposed to work or not. I can't do :nu $nu. It just gives an error.

BTW - I like how you highlighted the borders in double-lined borders. Nice work.

@fdncred fdncred merged commit 718ee3d into nushell:main Dec 1, 2022
@kubouch
Copy link
Contributor

kubouch commented Dec 1, 2022

For me :nu $nu works in an empty explore but I'm getting a panic on :nu.

@zhiburt Sorry for the ping, the steps to reproduce the panic:

> explore
# inside explore:
:nu
thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', crates/nu-explore/src/commands/nu.rs:87:41
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@fdncred
Copy link
Collaborator

fdncred commented Dec 1, 2022

oh, :nu $nu is working for me now too. I wonder what combination of things made it not work for me earlier.

but :nu enter doesn't fail for me in windows. lol

Oh oh, i see. If you type explore and the first thing you do is :nu it panics, but if you've done something like :nu $nu and then type :nu it doesn't panic for me.

LOL - I just found something else fun. Make sure you're in try mode and type clear. Not quite the behavior we want. haha

@zhiburt
Copy link
Contributor Author

zhiburt commented Dec 1, 2022

LOL - I just found something else fun. Make sure you're in try mode and type clear. Not quite the behavior we want. haha

Made it being ignored.

For me :nu $nu works in an empty explore but I'm getting a panic on :nu.

Ohhhhh sorry.

Must be fixed.

fdncred pushed a commit that referenced this pull request Dec 1, 2022
ref #6984

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@fdncred
Copy link
Collaborator

fdncred commented Dec 2, 2022

just fyi @zhiburt - i'm compiling information based on feedback from the team. i'll tag you in that issue when i create it.

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.

None yet

10 participants