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

hstr shouldn't use .bash_history as data source and not advertise to use "history -n" in PROMPT_COMMAND #400

Open
calestyo opened this issue Apr 21, 2020 · 20 comments

Comments

@calestyo
Copy link

calestyo commented Apr 21, 2020

Hey.

Not sure whether the following would really work in the end, but perhaps it's a better way of handling things.

AFAIU there are actually two histories...

  • the in-memory one of each shell process, which is what's actually used by bash itself, e.g. by the built-in-command history, bash's Ctrl-r or UP-key.
  • the history file (typically .bash_history), more or less "shared" by all open bash shell processes and either overwritten or appended to when a shell process quits (or when calling history with some parameters).

Also, AFAIU, there are basically two ways to use hstr:

  1. without PROMPT_COMMAND="history -a; history -n"
  2. or with

Each of them has problems associated:

(1):
When not using PROMPT_COMMAND="history -a; history -n", then newly entered commands in this shell won't get appended to the history file until the shell quits.
This also means, that any invocation of hstr won't see those commands, which is obviously bad.

(2):
When using PROMPT_COMMAND="history -a; history -n", then newly entered commands will get appended to the history file immediately.
While this solves the problem of not seeing newly entered commands of this shell it makes things pretty messy when using multiple shells:

Example, with two shells (the comment after each command is the order in which commands were entered in the two shells):
Shell I:

$ aaa #1
$ bbb #2
$ ccc #4

Shell II:

$ AAA #3
$ BBB #5

Looking at the actual .bash_history things look as expected:

aaa #1
bbb #2
AAA #3
ccc #4
BBB #5

But looking at the in-memory-histories of the two shells (which is what will also be used for e.g. the UP-key) things are pretty messed up:
Shell I:

 5032  aaa #1
 5033  bbb #2
 5034  ccc #4
 5035  ccc #4

Shell II:

 5032  AAA #3
 5033  bbb #2
 5034  AAA #3
 5035  BBB #5
 5036  BBB #5

Some entries are doubled, some are even missing. Strangely they're still missing when entering further commands (so it's not just one further missing reading of the history file), guess that's because of how bash appends/merges the history to/from the history file.

The whole thing can get even quite disastrous when people don't expect this behaviour, especially when e.g. multiple people use the same user account (role accounts like root), consider in chronological order e.g.:
User A: $ date
User A: # presses UP key, gets date again, presses ENTER
User B: $ cd tmp
User B: $ rm -rf *
User A: # presses UP key, gets date again, presses ENTER

It's not so unlikely, that A does this so fast (not expecting rm -rf might show up, that he actually executesrm -rf * wherever he is.

A partial solution for this might be to not advertise people to use:
PROMPT_COMMAND="history -a; history -n"
but rather just
PROMPT_COMMAND="history -a"
that way changes from other shells wouldn't be read in again (only for newly started shells).

This has however still the behaviour that commands are immediately appended to the history file, which has the following properties:
Since hstr uses the history file as data source, commands from other shells will show up immediately in this shell
This can be both, an advantage:

  • one will get commands from other shells in the history

and disadvantage:

  • one will get commands from other shells in the history (in the sense, it clutters mixes up the linear history (e.g. when using the raw-history view). Also it can again lead to unexpected results, when doing quick incremental searches.
    User A: might expect "Ctrl-r f ENTER" to execute his most recently entered "firefox --profile foo & exit", while he'll actually get User B's "rm -rf" which was more recent in the history file.

I think the disadvantage overweights by far,... it's effectively the same problem as above just with hstr and not the UP key or Bash's Ctrl-r (after all, a user might have kept Bash's Ctrl-r for use and bound hstr to another key).

I think the following could possibly solve this:
Don't use the history file at all as a data source, except perhaps when deleting commands from it.
Instead have someting like like "history | hstr --" bound to the key.
hsrt would then need to read the history from stdin (removing a portion of something like "^\(\*\|\)[[:digit:]]* *" from it's beginning - the history entry numbers might be prefixed with *).

The same would need to be done for directly invoking hstr, e.g. something like
alias hstr='history | hstr'

Has the advantage of not getting messed up with history file manipulations of other shells, while still seeing new commands in this shell.
No need to set any PROMPT_COMMAND at all, no need to sync the file after each command (which can easily cause defragmentation especially on CoW filesystems).

Deleting an entry would also need to be done via the buil-tin history -d offset and no longer affect the histories of other shells unless these are re-loaded.
But I think it would have the advantage of not having to write to the history file, which bash probably doesn't expect anyone elese to ever do.

Cheers,
Chris.

@calestyo calestyo changed the title hstr shouldn't use .bash_history as data source (except for deletion) and not advertise to use "history -n" in PROMPT_COMMAND hstr shouldn't use .bash_history as data source and not advertise to use "history -n" in PROMPT_COMMAND Apr 21, 2020
@calestyo
Copy link
Author

calestyo commented Apr 21, 2020

Oh and in addition:
I'm not really sure whether aliases/keybindings like history | hstr (i.e. ones that contain a pipe) are really safe in bash.

I cannot think of a counter example right now but maybe when the alias is then used in combination with other shell syntax like compound commands.
In any case one must of course prevent, that the history commands are all executed, which happened to me when I tried something like:
#do-not-execute----alias cat='cat < $(history) | tail'
(cat serving as a hstr replacement)

I forgot the "" around $(history) an the first history line was take as file for the redirect and the remainder of history was executed... while many of that will fail (since it starts with a number)... lines containing a & will cause that to be executed... lucky me, the first one was a & exit.

So perhaps it would be better to use constructs like:
hstr <(history)
that way, the worst, I can think of, that can happen is that something gets a fd with the history as content.

@dvorka
Copy link
Owner

dvorka commented Apr 21, 2020

Thank you for the comprehnesive analysis Christoph!

You are right, sending history command output to hstr is better solution. I actually planned to implement pipe support in the next release https://github.com/dvorka/hstr/milestone/12, however, I originally I didn't want to change how hstr works.

I will redesign hstr bash/zsh integration as you suggested in the next release. Overall I think that history-based design is much simpler and more robust (append & reload feels like hack, finding the right bash/zsh history file on different distros/installation is not straightforward and there are other problems as well).

I really appreciate your interest in hstr, time you invested into current design drawbacks analysis and making it Debian package. Thank you again!

@calestyo
Copy link
Author

Thanks for your kind words :-)

The problem with using the history file is generally that the shell probably doesn't expect anyone to do this. So there might be all kinds of weird bugs, e.g. when the shell truncates the file and so on.
I'd say even for deleting entries, hstr should directly edit the history file.

Before you invest a lot of time in redesigning, perhaps it makes sense to reach out for bash/zsh upstream, whether they'd be interested in you directly integrating hstr into the respective shell.
Of course I have no idea how much more of an effort that would be... but could have some advantages in terms of features.

Oh and btw: It wasn't me who packaged hstr for Debian, the maintainer is Daniel Echeverri.

@chewi
Copy link

chewi commented Nov 1, 2020

Great to know this is on the roadmap. Before finding this issue, I was thinking of a similar idea, sending just the in-memory history in through a pipe and appending that to the data from the history file. Not using the file directly at all seems cleaner though.

I want this because the idea of writing to my SSD every time I hit enter makes me twitchy. A silly concern, most likely, but hey.

@calestyo
Copy link
Author

calestyo commented Feb 17, 2021

just wondered, whether this is still considered? :-) @dvorka

@iamjackg
Copy link

Just to add to the conversation, there are more edge cases to keep in mind if you're going to implement this. The output of history can also contain an arbitrarily-formatted timestamp as defined by the HISTTIMEFORMAT env var.

I suggest taking a look at what fzf is doing to address this in their default CTRL+R binding: https://github.com/junegunn/fzf/blob/a4bc08f5a35da6e374a2ce0da4c931d7fd32fdf1/shell/key-bindings.bash#L47

For example, they use the fc builtin instead of the history builtin, which allows for omitting the line numbers and ignoring HISTTIMEFORMAT.

@calestyo
Copy link
Author

calestyo commented Jun 3, 2021

Isn't that also already the case when the history is read from the file?

@Gooberpatrol66
Copy link

Temporary workaround:
history -w ~/.bash_history.d/$$-temp && HISTFILE=~/.bash_history.d/$$-temp hstr -- && rm ~/.bash_history.d/$$-temp

@calestyo
Copy link
Author

But then one effectively looses persistent history?!

@Gooberpatrol66
Copy link

I have a somewhat elaborate setup where history is being read from somewhere else in bashrc and processed. I assumed that was the point of the thread, doing advanced things with the history and still having hstr work.

@dvorka
Copy link
Owner

dvorka commented Jan 11, 2022

@calestyo @Gooberpatrol66 Hey hello! Yes, I think that this feature makes sense (it might be either configurable option OR replace existing implementation), however, I'm unfortunately busy and don't have enough time to implement it. Apologies!

@akinomyoga
Copy link

akinomyoga commented Apr 3, 2023

We received a related report at ohmybash/oh-my-bash#422. The strange behavior that we observed there turned out to be caused by the settings instructed by hstr.

I'm not sure what is the original intent of hstr, but at least history -a; history -n won't work in a sane way unfortunately. It will duplicate entries of the current session randomly. It randomly picks up history entries added by other Bash sessions in a non-predictable way.

Possible solutions are:

  • If hstr wants to just reference the history entries from the history file, history -a is sufficient.
  • If hstr wants to also offer the shared-history feature, history -a; history -c; history -r should be used.

The harm of history -a; history -n and the trick history -a; history -c; history -r are described in the following links in detail:

#400 (comment)

I will redesign hstr bash/zsh integration as you suggested in the next release.

After three years, I still find history -a; history -n in the codebase. I'm not sure if hstr has completed the redesign or not, but if it would have been already completed and the history manipulation would not be needed anymore, could we just remove these hooks to PROMPT_COMMAND?

Overall I think that history-based design is much simpler and more robust

As described above, it is not robust and causes a problem, unfortunately.

@calestyo
Copy link
Author

calestyo commented Apr 4, 2023

Hey @akinomyoga.

Every time I've "met" you so far, you proved have extremely deep knowledge of bash and the stuff around it ... so most likely what you say is what should be done, but I don't quite understand it ^^

Possible solutions are:
If hstr wants to just reference the history entries from the history file, history -a is sufficient.

This is the "partial solution" I've looked at in the original report.

  • if really used alone (i.e. no history -r or history -n) it should not cause any messing with bash’s history (as nothing is re-read, unless you start a fresh new bash - where this is expected)
  • however, it as hstr reads in its data directly from the file it may affect that, with the possible advantages and disadvantages I've described above

If hstr wants to also offer the shared-history feature, history -a; history -c; history -r should be used.

  • same problems here, since the history file is always written, other instances of hstr (in other instances of bash) will be affected by that (which may have disadvantages)... and since history -r is done, couldn't that cause just the problematic behaviour I've described above? Another shell may overwrite the history file after the history -a from the current shell, the current would -read that of the other and then the history contents would (as described in the examples of my original report) contain unexpected entries.

I still think the main problem here is that hstr uses the history file as data source. It should use the "in-memory" history of the respective shell - and not use any history -X at all and never read/write the history file either.

@calestyo
Copy link
Author

calestyo commented Apr 4, 2023

Also, I don't think anything has been done in the meantime to get that fixed.

Overall I think that history-based design is much simpler and more robust

As described above, it is not robust and causes a problem, unfortunately.

What do you think would be the problem if using the output from history, i.e. the "in-memory" history?

@akinomyoga
Copy link

Overall I think that history-based design is much simpler and more robust

As described above, it is not robust and causes a problem, unfortunately.

What do you think would be the problem if using the output from history, i.e. the "in-memory" history?

Ah, OK. So "the history-based design" means the in-memory version but not the history -a; history -n approach? I meant the latter version of history -a; history -n. I agree that the in-memory version like history | hstr ... would be more robust.

@rpm5099
Copy link

rpm5099 commented Aug 24, 2023

This seems like a more appropriate issue to point out a really nice feature that I have heard many would find very useful - timestamps next to the command history indicating the most recent time the command was executed. This was also mentioned in #128, at least I think that's what they are asking for.

Reading this issue makes you realize the built in bash history is pretty limited considering the features that could be added to hstr if they were supported by it, such as:

  • storing first execution date, # times executed, etc., current working directory of execution
  • storing the result of execution if non-zero and coloring or hiding failed commands
  • any features that require information not stored in the history file

@calestyo
Copy link
Author

This doesn't seem to be related to the issue originally discussed here, or is it?

I guess it might make more sense to have this in a separate issue.

@rpm5099
Copy link

rpm5099 commented Aug 24, 2023

This doesn't seem to be related to the issue originally discussed here, or is it?

I guess it might make more sense to have this in a separate issue.

Because timestamps are not automatically stored in the bash history if the method of accessing and updating the history is re-tooled to in-memory methods then support for timestamps would likely need to be factored into that. I have not fully researched the functionality of bash history timestamps, so you could be correct @calestyo that this belongs in a separate issue.

@calestyo
Copy link
Author

btw: Until this is fixed in hstr, fzf and it's "Ctrl-R history mode" can be used as a somewhat similar alternative.

@dvorka dvorka unpinned this issue Feb 29, 2024
@Amain
Copy link

Amain commented Mar 16, 2024

To not have hstr append history items from each terminal immediately to disk (PROMPT_COMMAND="history -a; history -n), but read the history from the in-memory history of terminal it's running in, you can use something like:


function _hstr() {
    fc -ln -2147483648 |
      sed 's/^[ \t]\+ //' \
      >~/.hstr.tmp

    HISTFILE=~/.hstr.tmp hstr "${@}"
    rm - ~/.hstr.tmp
}

#export PROMPT_COMMAND="history -a; history -n; ${PROMPT_COMMAND}"
if [[ $- =~ .*i.* ]]; then bind '"\C-r": "\C-a _hstr -- \C-j"'; fi
# if this is interactive shell, then bind 'kill last command' to Ctrl-x k
if [[ $- =~ .*i.* ]]; then bind '"\C-xk": "\C-a _hstr -k \C-j"'; fi`

Notice the usage of _hstr in the bind command and the commenting of the PROMPT_COMAND.

fzf is good, but inferior to hstr for searching history conveniently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants