okbob / pspg

Unix pager (with very rich functionality) designed for work with tables. Designed for PostgreSQL, but MySQL is supported too. Works well with pgcli too. Can be used as CSV or TSV viewer too. It supports searching, selecting rows, columns, or block and export selected area to clipboard.
BSD 2-Clause "Simplified" License
2.48k stars 86 forks source link

Don't use the last searched pattern when doing a new search. #176

Closed rjuju closed 3 years ago

rjuju commented 3 years ago

Starting a new search is usually done when a new pattern is wanted, and this is also the normal behavior or other tools like vi. Continuing with the current pattern can be done using n/N, and any previous pattern can also be recalled using arrows when typing a pattern.

rjuju commented 3 years ago

Hi,

This has been bothering me for quite some time as I always type a few characters before realizing that the previous pattern was recalled. I'm assuming that most people would be confused too, so I simply removed the "last_search" related code. I can make it configurable if you prefer though.

okbob commented 3 years ago

čt 24. 6. 2021 v 10:31 odesílatel Julien Rouhaud @.***> napsal:

Hi,

This has been bothering me for quite some time as I always type a few characters before realizing that the previous pattern was recalled. I'm assuming that most people would be confused too, so I simply removed the "last_search" related code. I can make it configurable if you prefer though.

I think so it depends on the work and if user has strong experience with vi

I am not against this change. But I prefer to be configurable.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/okbob/pspg/pull/176#issuecomment-867447646, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO4YXXCX5RC46HK34S3TTULUNZANCNFSM47HMQZSA .

rjuju commented 3 years ago

I think so it depends on the work and if user has strong experience with vi

I mentioned vi, but I think this is the behavior of every single command-line interface tool I know (I just checked with less, more and bat).

I am not against this change. But I prefer to be configurable.

Fine by me. What behavior would you like by default? I think the "vi/less/more/bat/..." behavior should be default since it looks like the usual behavior, but I could understand that some people may be upset with this change after an upgrade.

okbob commented 3 years ago

čt 24. 6. 2021 v 12:54 odesílatel Julien Rouhaud @.***> napsal:

I think so it depends on the work and if user has strong experience with vi

I mentioned vi, but I think this is the behavior of every single command-line interface tool I know (I just checked with less, more and bat ).

I am not against this change. But I prefer to be configurable.

Fine by me. What behavior would you like by default? I think the "vi/less/more/bat/..." behavior should be default since it looks like the usual behavior, but I could understand that some people may be upset with this change after an upgrade.

It is hard to say, what is better in this case, The harder and more important is to make a name for a new option.

In this case we can be compatible with vi, less or more.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/okbob/pspg/pull/176#issuecomment-867539502, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO434MIDORSGHBQPRCDDTUMFFFANCNFSM47HMQZSA .

okbob commented 3 years ago

čt 24. 6. 2021 v 13:12 odesílatel Pavel Stehule @.***> napsal:

čt 24. 6. 2021 v 12:54 odesílatel Julien Rouhaud @.***> napsal:

I think so it depends on the work and if user has strong experience with vi

I mentioned vi, but I think this is the behavior of every single command-line interface tool I know (I just checked with less, more and bat).

I am not against this change. But I prefer to be configurable.

Fine by me. What behavior would you like by default? I think the "vi/less/more/bat/..." behavior should be default since it looks like the usual behavior, but I could understand that some people may be upset with this change after an upgrade.

It is hard to say, what is better in this case, The harder and more important is to make a name for a new option.

In this case we can be compatible with vi, less or more.

but I remember other dependency

some users uses "/" for repeated searching with empty string, and pspg uses "/" for cleaning after searching

In this perspective, I prefer current behaviour as default. Alternative behavior should be vi behaviour with repeating searching of previous pattern when entered pattern is empty

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/okbob/pspg/pull/176#issuecomment-867539502, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO434MIDORSGHBQPRCDDTUMFFFANCNFSM47HMQZSA .

rjuju commented 3 years ago

In this perspective, I prefer current behaviour as default.

Ok, new version of the patch does it this way:

I tested all permutation and it seems to work as expected. Note that I didn't add a toggle menu option for that, as it seemed overkill to add a new cmd_* for that.

Edit: sorry I forgot to amend the commit message, now done.

okbob commented 3 years ago

čt 24. 6. 2021 v 13:36 odesílatel Julien Rouhaud @.***> napsal:

In this perspective, I prefer current behaviour as default.

Ok, new version of the patch does it this way:

  • current behavior is default
  • new --no-last-row-search argument
  • new config option last_row_search

I tested all permutation and it seems to work as expected. Note that I didn't add a toggle menu option for that, as it seemed overkill to add a new cmd_* for that.

ok

this is just first point.

second point is reusing previous pattern, when user enter empty string (like vi)

You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/okbob/pspg/pull/176#issuecomment-867565390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO44U7UASV47ZAZUDDADTUMKDXANCNFSM47HMQZSA .

rjuju commented 3 years ago

second point is reusing previous pattern, when user enter empty string (like vi)

Oh wow, I didn't know it worked like that. I guess that a 2 keys solutions was never worthwhile to look for compared to a 1 key solution :)

Anyway, last push-force should implement it. Note that it's only done in the last_row_search = false mode.

okbob commented 3 years ago

Merged, thank you for patch

rjuju commented 3 years ago

Thanks a lot!

okbob commented 3 years ago

https://github.com/okbob/pspg/releases/tag/5.0.5

rjuju commented 3 years ago

Nice, thanks a lot Pavel. And sorry for missing the required changes in the README.

okbob commented 3 years ago

so 26. 6. 2021 v 6:56 odesílatel Julien Rouhaud @.***> napsal:

Nice, thanks a lot Pavel. And sorry for missing the required changes in the README.

It is no problem :)

You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/okbob/pspg/pull/176#issuecomment-868948097, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO45O4N6F2SUBOBZJPG3TUVMXTANCNFSM47HMQZSA .