AMythicDev / minus

An asynchronous, runtime data feedable terminal paging library for Rust
https://crates.io/crates/minus/
Apache License 2.0
317 stars 23 forks source link

Feat: Add following mode #114

Closed TornaxO7 closed 9 months ago

TornaxO7 commented 9 months ago

Closes https://github.com/arijit79/minus/issues/113 Closes https://github.com/arijit79/minus/issues/81

I haven't added any docs yet because I'm unsure how I can test it and I'd like to get some suggestions from you first, like if it's fine how I changed the code :)

AMythicDev commented 9 months ago

The implementation seems to be ok however there are still a few things left:-

I think this PR should also close #81.

TornaxO7 commented 9 months ago

@arijit79 update :D (I tackled your points)

AMythicDev commented 9 months ago

You missed my point actually. I was saying that in static output, the pager should jump to the last line only if follow mode is enabled so that it matches with the functionality of dynamic mode. Also your code fails to compile. Also the ToggleSearch keybinding still does not work: its likely you didn't add it in the newer input system. I am making a commit to fix these issues.

AMythicDev commented 9 months ago

Now everything seems to be in place. Just few more things to get done. First if the user enables follow mode and new data is coming, minus takes them to the last line of output. But if no new input comes, then there it does not jump directly towards the last line.

AMythicDev commented 9 months ago

Thanks for 9e9b273. I didn't even notice that.

EDIT: BTW I am working on the issue that I talked in my earlier comment. I will post it later today.

TornaxO7 commented 9 months ago

EDIT: BTW I am working on the issue that I talked in my earlier comment. I will post it later today.

oh eh, I already pushed a change, which, if I understood you correctly, should address the issue. :sweat_smile:

AMythicDev commented 9 months ago

No that wasn't exactly what I was talking about. I had already pushed a commit fixing exactly the same issue that you pushed. Don't worry I will fix the issue today.

TornaxO7 commented 9 months ago

No that wasn't exactly what I was talking about. I had already pushed a commit fixing exactly the same issue that you pushed. Don't worry I will fix the issue today.

oh, sorry... so should I let you do the changes for the time being?

AMythicDev commented 9 months ago

I have pushed the changes. You can try them to report any errors/bugs that you find.

TornaxO7 commented 9 months ago

I have pushed the changes. You can try them to report any errors/bugs that you find.

May I ask how I can test it out? Do I have to implement a custom binary which uses the lib?

AMythicDev commented 9 months ago

You can try it out in any of the examples.

You can run:-

cargo run --examples [example name] --features [features that you want]

For example:-

cargo run --example dyn_tokio --features=dynamic_output,search
TornaxO7 commented 9 months ago

I've got an idea: Would be neat, if the statusbar could show an indicator that it's currently in "following-mode".

AMythicDev commented 9 months ago

I was thinking about the same while writing the code and I have planned for it. We could use a traditional solution right now But it would get much easier to implement once I introducecommand queue feature which would allow us to chain another command after the current command is executed. This would take me a day to get there.

Other then that I think this PR is ready so if you want this immediately on the main branch, I can merge this otherwise I will keep this until I get the status bar message thing going.

TornaxO7 commented 9 months ago

whoops.... I read your message too late ;-; I can revert the changes if you want.

TornaxO7 commented 9 months ago

I was thinking about the same while writing the code and I have planned for it. We could use a traditional solution right now But it would get much easier to implement once I introducecommand queue feature which would allow us to chain another command after the current command is executed. This would take me a day to get there.

Sounds good!

Other then that I think this PR is ready so if you want this immediately on the main branch, I can merge this otherwise I will keep this until I get the status bar message thing going.

I don't need it now, so you can implement the command queue first

AMythicDev commented 9 months ago

It's good but I was thinking about using a message rather than adding an entire block to show that. If static text is required we could also use a single character for that. What do you say?

TornaxO7 commented 9 months ago

It's good but I was thinking about using a message rather than adding an entire block to show that.

hm... may I ask what you mean with "message"?

If static text is required we could also use a single character for that. What do you say?

On the one hand I'd prefer to have this static text because it's self-explaining what it indicates. If there will be more entries, a single letter would be fine but on the other hand a [F] might be already self-explaining I think, so yeah, I think something like [F] is fine.

AMythicDev commented 9 months ago

A message in minus allows you to show a text message to the user at the prompt site. It would disappear when the user presses any other key/does a mouse action. We can display a message like Follow output enabled when it is enabled. If you want to know more about a message, try msg-tokio example to see a practical demo of it.

TornaxO7 commented 9 months ago

A message in minus allows you to show a text message to the user at the prompt site. It would disappear when the user presses any other key/does a mouse action. We can display a message like Follow output enabled when it is enabled. If you want to know more about a message, try msg-tokio example to see a practical demo of it.

oh ok. So, I'd prefer a static text because assuming you're looking through some logs and you forgot that the follow mode is still enabled, you'll start scrolling up but after some time something new appears and you'll be thrown back to the bottom. I could imagine that this might be a bit annoying.

AMythicDev commented 9 months ago

Hmm. Fine then.

TornaxO7 commented 9 months ago

but yeah, now I'm quite happy with the result :+1:

thank you for being very responsive :)

AMythicDev commented 9 months ago

Let's merge this then. I will introduce minor improvements on this in future commits.