Remove all history manipulation #117
Conversation
|
Can you detail exactly what behavior differences users will observe with this change, and why you feel they are acceptable? A notable regression with this change appears to be that pipelines will no longer be captured properly (e.g. if you run |
|
@gaelicWizard thanks for the initiative with the PR and @dimo414 thanks for chiming in. My sentiment on this change is the same as mentioned prior in: #115 (comment) I don't believe the regression of not capturing multiple commands is an acceptable one and favor the trade off here with overriding history to get the command line as its typed. With that said, @gaelicWizard if you don't require $1 on preexec, feel free to to remove the history manipulation as mentioned #48 (comment). If there's another way to get access to the command exactly as it's typed without history in Bash I'd love to update this! |
|
Sorry, this PR is marked draft because I both haven't committed everything nor put in any explanation. After reading other open issues and PRs, I'm going to actually try to just make the history manipulation conditional rather than remove it. It's now clear to me that it's necessary for a particular use case given constraints. My motivation is twofold: (1) don't change things behind my back; (2) allow hook to run before each command, rather than before each command line. |
That's totally reasonable to want, but it's not how bash-preexec is implemented (by design). Are you sure you even want to use bash-preexec? Maybe you just want to implement your own DEBUG trap? |
|
Lol, I can't use bash-preexec and a custom debug trap unless I write a wrapper with conditional behavior and...that's why we use bash-preexec: so we get the benefit of each other's improvements. I'm not the first one to want to be able to use debug hooks along with bash-preexec. See #52, from someone who uses bash-preexec and is asking for DEBUG traps to run for every command. |
gaelicWizard
force-pushed
the
history
branch
2 times, most recently
from
612a512 to
d08e109
Compare
|
Alsö, #109 would run the preexec hook multiple times on purpose. |
Warn the user on stderr if we need to modify `$HISTCONTROL`, but first check if our warning should be suppressed by the user setting `$__bp_suppress_histcontrol_warning` to something.
When determining the command line, check if `$HISTCONTROL` is configured as expected. If not, then fall back to the built-in variable `$BASH_COMMAND` for a second-best command line. This accounts for when the user either changes `$HISTCONTROL` after loading `bash-preexec`, or otherwise removes/disables `__bp_adjust_histcontrol`.
|
I'm closing this PR and will open a whole new one based on #96 , which does 98% of what I need. It requires Bash version 5 or higher, but @cornfeedhobo made a comment that makes it not need v5 so I'm just doing that. |
Remove mangling of HISTCONTROL and remove use of history as a mechanism for detecting the "current" command. Instead, use $BASH_COMMAND like Jesus intended.