history substring search plugin from fizsh

[sunaku]

This commit imports the history substring search script (which
adds Fish shell like history search to ZSH) from fizsh-1.0.1 as an oh-my-zsh plugin.

[sorin-ionescu]

I have found a very annoying bug. Newline, tab and other special characters are not escaped.

~ ❯ echo "Hello\nWorld"
Hello
World

When I hit key up to go back in history, the output is wrong.

~ ❯ echo "Hello
World”

While this is a benign example, this behaviour can result in many syntax errors and head scratching.

[sunaku]

Thanks for your report! I forwarded it to Guido van Steen, the author of the history plugin.

[sunaku]

@sorin-ionescu did Guido van Steen respond to your bug report?

[sorin-ionescu]

Since he hasn't replied to this issue, I presume not.

[sorin-ionescu]

I have figured it out. I'll use the following code as an example.

for i in $(jot 5); do
    echo "$i: Hello\nWorld"
done

It gets stored as is in ~/.zsh_history and properly retrieved using ZSH's native methods. However, if one executes history, it gets printed as the following.

for i in $(jot 5); do\n    echo "$i: Hello\nWorld"\ndone

This plugin retrieves the line above using fc -ln and replaces \n with new line to attempt to reprint in the multiline, original format. In this case, it is not possible to differentiate between the \n in Hello\nWorld and an \n found elsewhere.

The solution to this bug is to use a different method to retrieve history.

[sunaku]

Good find! I noticed that Ctrl-R (bck-i-search) does not suffer from this problem.

[sunaku]

@sorin-ionescu, I changed the retrieval mechanism to index the $history array directly instead of going through the fc builtin in sunaku/oh-my-zsh@c7a8fab. This seems to fix the problem for me, please verify.

[sorin-ionescu]

I did that too about two hours ago. After I found the fc problem, I went into #ZSH on Freenode and someone suggested to try $history, which I did. I was going to push it tomorrow after some testing. :p

You can delete the comment on line 89. That said, I don't know what are $history's limitations and why the author used fc in the first place.

[sunaku]

I'm glad we arrived at the same solution. I also don't know why Guido chose fc over $history. He hasn't responded to my emails for a while now so perhaps we can take up this plugin's maintenance: I noticed some repeated code in the then/else clauses that needs to be DRY'ed up. :)

[sorin-ionescu]

Yes, I have noticed that he doesn't respond to email, which prompted me to read the code and fix the problem myself. I have not looked at the code in detail beyond the necessary lines to fix the history retrieval bug, but I have noticed that there is massive duplication that should be refactored. I have created a repository at zsh-history-substring-search for you to fork, make changes, and submit pull requests. I have already made a couple of changes.

[brentp]

This seems to be working well for me. Any reason it's not getting pulled?

[vguerci]

Thanks guys for this pull request, just tested it and it is really nice, I like the zsh up-history but I was "dreaming" about something closer to ctrl+R, great! :) Would love to see that merged in master!

[sorin-ionescu]

@vguerci: I use vi key bindings. So, I'm not familiar with CTRL+R. Do you care to elaborate?

@sunaku: Sync this pull request with zsh-history-substring-search please.

[vguerci]

@sorin-ionescu ctrl-R is the default shortcut in many shells for history backward search, which is really similar to that plugin, but less user friendly imho. See history-incremental-search-backward in zsh documentation.

[sorin-ionescu]

@brentp There is a lot of code duplication. It should still be cleaned up for easier maintainability before pulled. Volunteers?
@vguerci Oh, I have it mapped to a different binding. See issue #211.

[sunaku]

@sorin-ionescu I looked at your repo and pulled in the relevant changes in commit 5f9418363add4589d93c6729a8beeba026422985. I am refactoring the code duplication now.

[sunaku]

I'm finished with refactoring and am quite happy with the resulting code now. Enjoy! :]

[sorin-ionescu]

I'll look at your changes. One thing I did was to rename it to the more descriptive history-substring-search since ZSH already has history search.

[sunaku]

Done, renamed the plugin according to your preference.

[vguerci]

Merged on my local branch, looks like working perfectly, good job.

Just found a small visual glitch, which I presume comes from original version, highlights stays even if command is deleted/edited, it reappears on newly entered command.

[sorin-ionescu]

@sunaku Can you do a pull request on my zsh-history-substring-search repository for me to test it? Thank you.

[sunaku]

@vguerci, fixed by always calling _zsh_highlight-zle-buffer in history-substring-search-highlight() like the original script. Bug was introduced by me. :-/

[sunaku]

Alright Sorin, after a lot of work, I have a disjoint fork of your repository. (Note that I previously had my own repo with the same name so GitHub didn't let me fork yours until I deleted mine.) Cheers.

[sunaku]

Phew! Finally got this pull request synced with OMZ master and my disjoint fork created for Sorin. No more rebasing for today, I hope! :]

[sorin-ionescu]

You removed the symlink, which screwed the merge up. I merged it. I'll test it over the next few days. So far, I noticed that colour highlighting doesn't work.

[sunaku]

Sorin, I didn't remove the symlink: rather, the symlink never existed in my fork's history in the first place. That's what I meant by "disjoint".

I don't see any issues with syntax coloring, everything seems normal. Try my OMZ fork's personal or history branch for example.

[sorin-ionescu]

You have renamed the variables, which were called F_ordinary_highlight and F_out_of_matches_highlight before, but that should not be the problem. I'll check if you have made a change that conflicts with zsh-syntax-highlighting.

[sorin-ionescu]

It doesn't seem to be conflicting with zsh-syntax-highlighting.

At the end of the history-search-highlight, you have region_highlight=("$history_substring_search_my_mbegin $MEND $1"). What do you do with it? It seems unused.

[sunaku]

That line applies a highlight (specified by $1) to the part of the history command that matches the user's input.

[sorin-ionescu]

I was saying that the variable region_highlight is never used. The function is called, but I see no highlighting.

[sunaku]

Hmm, squashing all commits eh? That would create two copies of the code (one without adequate history, due to squashing) that could go out of sync. I wonder if we should keep this plugin separate from OMZ, just like the zsh-syntax-highlighting plugin is?

[sorin-ionescu]

Is the history relevant for the initial commit? It's up to you if you want it independent. Right now, it is independent.

[sunaku]

You're right. I think I would prefer it to be independent, because there is a long turn-around time for getting pull requests merged into OMZ and we wouldn't want to bother you guys every time there are changes in this plugin.

So I'm going to close this pull request. Guido, do you approve?

[sorin-ionescu]

Independent development doesn't mean no pull requests. It just means that OMZ doesn't need to know the entire history of the plugin between pull requests, only that the version has been bumped in a squashed commit.

[sunaku]

Alright, I have prepared the single, giant squashed commit for merging into OMZ as you requested.

[guidovansteen]

If possible I would like to keep the history as well. I guess the previous commits can still be found in your upstream repository!?

[sunaku]

Yes, that is correct Guido.

[sorin-ionescu]

@robbyrussell, let's get this merged.

[ColinHebert]

This looks really great.

But just a question, wouldn't it be more convenient to keep this plugin as a git submodule? This way the history is kept and it's easy to upgrade to any version/fork/branch of the module.

[sorin-ionescu]

@ColinHebert, no it would not be convenient. Submodules are complicated and @sunaku is git rebase trigger happy.

[sunaku]

@sunaku is git rebase trigger happy.

LOL well said, sir! That made my day! :-)

[sorin-ionescu]

Why have you renamed the functions to history-substring-search-up and history-substring-search-down? Keep it consistent with the rest of ZSH by using the terms backward and forward like history-incremental-search-backward and history-incremental-search-forward.

[guidovansteen]

@sorin-ionescu: This change has been discussed before. We have renamed backward and forward because these names are confusing. up and down refer to the buttons pressed, and are therefore less prone to misunderstandings. Moreover, up and down are also used in some of zsh's standard widgets, such as up-line-or-history and down-line-or-history. So I am against reverting this.

[guidovansteen]

I have thought about it one more time. up-line-or-history and friends are widgets used for editing, and not for searching. So the argument that they are standard zsh widgets is not that convincing. For this reason I might be willing to revert the name change. It would depend on Sunaku's feelings about this though.

@sorin-ionescu: You say that reverting the name change would make things consistent with oh-my-zsh. The argument that the name change makes the script inconsistent with the current state of zsh would have been more convincing to me. Also, please refrain from using blunt imperatives, when addressing other people. That is etiquette too!

[ghost]

+1

[sunaku]

I prefer the current up & down naming. It makes the implementation easier to understand.

[guidovansteen]

I have looked at the implementation. I agree with Sunaku: the names up and down make things easier to understand.

IMO the up and down naming is quite defensible as well, because the widget is not exclusively intended for history-searches.

[sorin-ionescu]

I have done a listing of functions that contain the words backward/forward and up/down for reference.

accept-line-and-down-history
down-case-word
down-history
down-line-or-history
down-line-or-search
emacs-forward-word
forward-char
forward-word
history-beginning-search-forward
history-incremental-pattern-search-forward
history-incremental-search-forward
history-search-forward
up-case-word
up-history
up-line-or-history
up-line-or-search
vi-down-line-or-history
vi-forward-blank-word
vi-forward-blank-word-end
vi-forward-char
vi-forward-word
vi-forward-word-end
vi-history-search-forward
vi-up-line-or-history

[guidovansteen]

history-substring-search is a compound widget. It performs both history-searching and line-editing. That is why it is impossible to apply Zsh's naming conventions in a consistent way anyway.

The widget is modeled after Fish, where it is bound to up and down. Therefore let's stick to the current names.

[robbyrussell]

Merged this... let's update the Plugins wiki page accordingly. Am trusting @sorin-ionescu's judgement on this one

[ghost]

@robbyrussell - It got put in the root of the oh-my-zsh project and not in the plugin directory
Fixed in #489

[sunaku]

@dreur is correct. That was my fault: when I squeezed all the commits together, I forgot to move the files into the plugins subdir.

[sorin-ionescu]

 source "${0:h}/history-substring-search.zsh"

[guidovansteen]

Out of curiosity: Is the version included in oh-my-zsh the same as the main one being developed at https://github.com/zsh-users/zsh-history-substring-search?

[sorin-ionescu]

It should be the same.

[sorin-ionescu]

Even better:

source "${0:r:r}.zsh"

Memorise:

:e --> extension
:h --> head (dirname)
:t --> tail (basename)
:r --> rest (extension removed)

[sunaku]

Thanks Sorin, your change is applied at zsh-users/zsh-history-substring-search@9d3683b.

[sorin-ionescu]

I'm having a lot of hard time going up a line in multi-line commands. Sometimes, it works, but the majority of time, it does not but goes back in history.

[guidovansteen]

I have not noticed any problems with editing multi-line commands
(except slowness). Could you provide an example history file that
causes a bug?

On Sun, Sep 4, 2011 at 7:17 PM, sorin-ionescu
[email protected]
wrote:

I'm having a lot of hard time going up a line in multi-line commands. Sometimes, it works, but the majority of time, it does not but goes back in history.

Reply to this email directly or view it on GitHub:
#215 (comment)

[vguerci]

@sorin-ionescu maybe same issue : zsh-users/zsh-history-substring-search#2