WIP: Add newline support #162
Conversation
|
@cantino I'd like your thoughts on this before I go further, not sure if draft pull requests make a notification or not. I now better understand the |
|
Sorry for the delay @shelvacu! You're correct about I think it's entirely possible that the current logic to get history into both the shell history and the mcfly database is crufty / unnecessarily complex. It's evolved over time as I worked on this and I've never really revisited it. |
| @@ -21,8 +21,7 @@ MCFLY_SESSION_ID="$(command dd if=/dev/urandom bs=256 count=1 2> /dev/null | LC_ | |||
| export MCFLY_SESSION_ID | |||
|
|
|||
| # Find the binary | |||
| MCFLY_PATH=${MCFLY_PATH:-$(which mcfly)} | |||
There was a problem hiding this comment.
There was definitely a reason for $MCFLY_PATH to exist, but I can't recall anymore what it was.
There was a problem hiding this comment.
Just leave it out then? I can't find anything that reads it, just things setting it in the dev.* scripts.
| # Run mcfly with the last history entry on stdin. It will: | ||
| # * Append the command to $HISTFILE, (~/.bash_history by default) | ||
| # * Parse out the command and and save it to the database. | ||
| HISTTIMEFORMAT="%s:" history 1 | mcfly add --exit $exit_code --command-from-stdin |
There was a problem hiding this comment.
Does this still work on bash <4?
There was a problem hiding this comment.
According to the bash release notes, HISTTIMEFORMAT was added in Bash 3, released in 2004. I did a quick test to be sure, it behaves the same as bash 4. Should be good.
|
@shelvacu What's the status of this PR now that @cantino has left a few comments? From your description I can't tell if there is more to be done. You mention that as-is, the PR does correctly handle multi-line commands, but also state that you were sharing the PR as a draft before going further. What is left? Perhaps I can help? This bug is a particular nuisance to my workflow as I tend to lean heavily on multi-line commands. |
So I've been working on #50 and it sort of feels like things are sprawling out more and more, and I wanted to both contribute what I'd done so far and get feedback before making more changes.
As it stands right now, this PR does correctly record commands-with-newline(s) into the database.
The first issue I ran into was that the search interface would not show newlines correctly. I believe I've fixed it, but I haven't tested it rigourously.
While developing, I noticed that the search was running the wrong version of mcfly (installed version in
.cargo/binrather than the development version intarget) despite usingdev.bash, because the history addendum uses$MCFLY_PATHwhereas the bind command just callsmcfly. I was unable to wrassle the multi-level quoting into working correctly in the bind command with a variable substitution, so I instead removed$MCFLY_PATHand relied onmcflycalling the right binary. This may not be desireable.In changing the way the command is read in from history, I cut out some of the code that wrote to a file, filtered, and wrote back out history to then be re-read by
history -cr. I think for bash the best solution would involve no itermediary files, just the database andhistorycalls.There seems to be some logic for removing history entries starting with
#mcfly, but I don't see that in my history anyway so I don't understand why it's there. As it is in this PR, that functionality is never used I think.Also worth noting that currently this is for bash only. I haven't looked into zsh or fish at all.
Let me know your thoughts.