search

cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.85k stars 178 forks source link

WIP: Add newline support #162

Open shelvacu opened 3 years ago

shelvacu commented 3 years ago

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/bin rather than the development version in target) despite using dev.bash, because the history addendum uses $MCFLY_PATH whereas the bind command just calls mcfly. 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_PATH and relied on mcfly calling 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 and history calls.

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.

shelvacu commented 3 years ago

@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 #mcfly: functionality; In bash >= 4 it is possible to get a full multiline search from $READLINE_LINE, but for earlier bash I assume it will just mess up a multi-command when you press C-r

cantino commented 2 years ago

Sorry for the delay @shelvacu! You're correct about #mcfly: and bash versions <4.

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.

cannonpalms commented 2 years ago

@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.