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

Some commands not showing up and inaccurate command duration #1003

Open
gvlassis opened this issue May 23, 2023 · 13 comments
Open

Some commands not showing up and inaccurate command duration #1003

gvlassis opened this issue May 23, 2023 · 13 comments

Comments

@gvlassis
Copy link

I am using freshly installed Atuin v14.0.1 through Homebrew on bash+macOS.

I have encounterd two weird issues that might or might not be related:

  1. Sometimes a command does not show up in history (see "fastfetch" command in first screenshot). I have not noticed any pattern, even though there might be one.
  2. Sometimes the duration of a command is inaccurate (see "which atuin" in second screenshot). Also, it sometimes shows up as 0 on the first invocation of the interactive history search and then as something else on the following invocations (compare "which atuin" in first and second screenshots).
    Screenshot 2023-05-23 at 12 49 22 PM
    Screenshot 2023-05-23 at 12 49 49 PM
@conradludgate
Copy link
Collaborator

Hmm. That's very weird. What shell do you use?

@gvlassis
Copy link
Author

bash 5.2, installed through Homebrew. The terminal is Kitty, but this also happens with Terminal.app (the preinstalled terminal emulator on macOS).

@gvlassis
Copy link
Author

I just confirmed that this happens to me even if:

  1. I use the installation script
  2. .bashrc only contains the two lines added by the installation script

@gvlassis
Copy link
Author

This does NOT happen with ble.sh (https://github.com/akinomyoga/ble.sh) installed. With some trial and error, I identified:

precmd_functions+=(_atuin_precmd)
preexec_functions+=(_atuin_preexec)

, which are returned from atuin init bash, as the problematic lines.

@danwilliams
Copy link

danwilliams commented Nov 10, 2023

I have a brand-new installation on Bash, and I am finding that lots of my commands do not end up in history. Very odd.

I will run a command, hit up arrow to run it again - and it's not there. I have up arrow disabled for Atuin at present, too, but the command also doesn't show int he Atuin interface with Ctrl-R.

There is no pattern to this and if I run it again the command will generally then show up.

The problem is it leads to very unreliable behaviour and slows down shell interaction as you have to check what's about to be executed and not rely on your memory.

@danwilliams
Copy link

I think I have found by usage an observation that the problem happens consistently when using Ctrl-R - the command chosen by that process never seems to be added afresh to the current history context, meaning that it's not available when you press Up Arrow. I'm not sure if it happens in other ways as well, but I will continue to observe.

@arcuru
Copy link
Sponsor Contributor

arcuru commented Nov 10, 2023

@danwilliams You haven't said, but I suspect that you are using enter_accept and you are using Atuin v17.0.0.

v17.0.1 has #1342, which will ensure that the command you select using enter_accept from the Atuin interactive search will show up in the atuin history.

I've opened #1383 to make sure that those commands will be added to the bash history as well so that your up-arrow usage will also work.

You can use both these fixes today by putting the code of atuin init bash --disable-up-arrow into .bashrc and editing the __atuin_history function to the version in the #1383 PR.

@danwilliams
Copy link

@arcuru thanks for the super-fast and extremely helpful response 🙂 You're right, I should have mentioned the version. I simply did cargo install atuin and have checked now - I'm on v17.0.1, which is interesting. I've checked the manual so that I understand what enter_accept means and yes you are correct, I am using that.

I am not sure if #1342 is working okay, because I'm meant to have that fix and yet it seems commands are not showing up in Atuin's history. I will keep an eye on that, as it's the up-arrow behaviour that is the most noticeable. I'm not sure if I've seen them missing from Atuin's history since applying --disable-up-arrow.

Thank you for opening #1383 - I am already using atuin init bash --disable-up-arrow and that works fine (I find the Atuin up-arrow behaviour very odd and there's seemingly no way to restrict it to a terminal session, so I disabled it). I don't know if/where I can edit the atuin.bash file under a normal installation, so I cloned the repository and installed from source with your fix from the PR. I can confirm that doing that instantly fixes the up-arrow problem ❤️

I will keep an eye on the Atuin history issue to see if I can confirm that either way.

Thanks again!

@danwilliams
Copy link

danwilliams commented Nov 11, 2023

Yup, unfortunately #1342 is not fully fixed, as some newly-typed commands are definitely not getting added to Atuin's history (they are in Bash history and I can access them through up-arrow).

#1383 is indeed fixed though, and has not been a further problem - hitting enter on the Ctrl-R list does add to Bash history and up-arrow availability.

@danwilliams
Copy link

I've noticed two types of command that don't show up in Atuin's log:

  • Compound commands, e.g. cargo build && cargo clippy && cargo doc
  • Commands that use git filter-branch - I have no idea why that would be the case; I have two that I've tried and one shows up whereas one doesn't. Both are accessed via short names so are not long complex commands. I'm trying to work out more to report on this.

@danwilliams
Copy link

Interactive commands don't seem to get added - e.g. nano. (Talking about the Atuin history here.)

akinomyoga added a commit to akinomyoga/atuin that referenced this issue Jan 6, 2024
This fixes the second issue of 0s or wrong command duration reported
at atuinsh#1003.

The problem is that, with bash-preexec, the preexec hook may be called
even for the commands executed by the keybindings.

* In particular, the preexec is called before the command
  `__atuin_history` is executed on pressing [C-r] and [up].  In this
  case, $1 passed to __atuin_preexec contains the last entry in the
  command line, so `atuin history start` is called for the last
  command.  As a result, pressing [C-r] and [up] clears the duration
  of the last command.  This causes the reported 0s duration.

* Furthermore, the precmd hook corresponding to the keybinding command
  will not be called, so the duration is only filled when the next
  user command starts.  This replaces the duration of the last command
  with the time interval between the last press of [C-r] or [up] and
  the start time of the next command.  This causes the reported wrong
  duration.

There is no general and robust way to distinguish the preexec
invocation for keybindings from that for the user commands, but in
this patch we exclude the preexec invocation for Atuin's keybindings
[C-r] and [up] at least.
akinomyoga added a commit to akinomyoga/atuin that referenced this issue Jan 6, 2024
This fixes the second issue of "0s or wrong command duration" reported
at atuinsh#1003.

The problem is that, with bash-preexec, the preexec hook may be called
even for the commands executed by the keybindings.

* In particular, the preexec is called before the command
  `__atuin_history` is executed on pressing [C-r] and [up].  In this
  case, $1 passed to __atuin_preexec contains the last entry in the
  command line, so `atuin history start` is called for the last
  command.  As a result, pressing [C-r] and [up] clears the duration
  of the last command.  This causes the reported 0s duration.

* Furthermore, the precmd hook corresponding to the keybinding command
  will not be called, so the duration is only filled when the next
  user command starts.  This replaces the duration of the last command
  with the time interval between the last press of [C-r] or [up] and
  the start time of the next command.  This causes the reported wrong
  duration.

There is no general and robust way to distinguish the preexec
invocation for keybindings from that for the user commands, but in
this patch we exclude the preexec invocation for Atuin's keybindings
[C-r] and [up] at least.
akinomyoga added a commit to akinomyoga/atuin that referenced this issue Jan 6, 2024
This fixes the second issue of "0s or wrong command duration" reported
at atuinsh#1003.

The problem is that, with bash-preexec, the preexec hook may be called
even for the commands executed by the keybindings.

* In particular, the preexec is called before the command
  `__atuin_history` is executed on pressing [C-r] and [up].  In this
  case, $1 passed to `__atuin_preexec` contains the last entry in the
  command history, so `atuin history start` is called for the last
  command.  As a result, pressing [C-r] and [up] clears the duration
  of the last command.  This causes the reported 0s duration.

* Furthermore, the precmd hook corresponding to the keybinding command
  will not be called, so the duration is only filled when the next
  user command starts.  This replaces the duration of the last command
  with the time interval between the last press of [C-r] or [up] and
  the start time of the next command.  This causes the reported wrong
  duration.

There is no general and robust way to distinguish the preexec
invocation for keybindings from that for the user commands, but in
this patch we exclude the preexec invocation for Atuin's keybindings
[C-r] and [up] at least.
ellie pushed a commit that referenced this issue Jan 6, 2024
)

This fixes the second issue of "0s or wrong command duration" reported
at #1003.

The problem is that, with bash-preexec, the preexec hook may be called
even for the commands executed by the keybindings.

* In particular, the preexec is called before the command
  `__atuin_history` is executed on pressing [C-r] and [up].  In this
  case, $1 passed to `__atuin_preexec` contains the last entry in the
  command history, so `atuin history start` is called for the last
  command.  As a result, pressing [C-r] and [up] clears the duration
  of the last command.  This causes the reported 0s duration.

* Furthermore, the precmd hook corresponding to the keybinding command
  will not be called, so the duration is only filled when the next
  user command starts.  This replaces the duration of the last command
  with the time interval between the last press of [C-r] or [up] and
  the start time of the next command.  This causes the reported wrong
  duration.

There is no general and robust way to distinguish the preexec
invocation for keybindings from that for the user commands, but in
this patch we exclude the preexec invocation for Atuin's keybindings
[C-r] and [up] at least.
@cenuij
Copy link

cenuij commented Feb 16, 2024

Yup, unfortunately #1342 is not fully fixed, as some newly-typed commands are definitely not getting added to Atuin's history (they are in Bash history and I can access them through up-arrow).

In my experience, whenever I use Ctrl-R and then ESC (i.e. I don't select anything), whatever command I then type on the command line does not get added to the history. However, if I do Ctrl-R, ESC and then ENTER (i.e blank command) before I type a new command, it works.

Also, when I use Ctrl-R and then TAB to edit the command, the edited command is also not added. I think it should be added?

@atuin-bot
Copy link

This issue has been mentioned on Atuin Community. There might be relevant details there:

https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175/14

akinomyoga added a commit to akinomyoga/atuin that referenced this issue Feb 17, 2024
In GitHub atuinsh#1509, we blocked the unintended preexec caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

atuinsh#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
atuinsh#1003 (comment)
atuinsh#1727
atuinsh#1728
akinomyoga added a commit to akinomyoga/atuin that referenced this issue Feb 17, 2024
In GitHub atuinsh#1509, we blocked the unintended preexec caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

atuinsh#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
atuinsh#1003 (comment)
atuinsh#1727
atuinsh#1728
akinomyoga added a commit to akinomyoga/atuin that referenced this issue Feb 17, 2024
In GitHub atuinsh#1509, we blocked the unintended preexec event caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

atuinsh#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
atuinsh#1003 (comment)
atuinsh#1727
atuinsh#1728
ellie pushed a commit that referenced this issue Feb 17, 2024
In GitHub #1509, we blocked the unintended preexec event caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
#1003 (comment)
#1727
#1728
ellie pushed a commit that referenced this issue Feb 26, 2024
In GitHub #1509, we blocked the unintended preexec event caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
#1003 (comment)
#1727
#1728
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

No branches or pull requests

6 participants