search

zsh-users / zaw

zsh anything.el-like widget.
BSD 3-Clause "New" or "Revised" License
572 stars 67 forks source link

Add yudai code #61

Closed willghatch closed 9 years ago

willghatch commented 9 years ago

This merges in a bunch of code from the fork of @yudai. Several contributors to zaw are using this code in their forks, and at least one pull request submitted to the main zsh-users repo requires some of these commits. This commit represents a merger of Yudai's code up to his latest 5 commits, which add features that aren't all working for me.

willghatch commented 9 years ago

I don't know why I made a pull request. Instead I just pushed to master, since I can do that now, and this code is already in use by several people, so it's been reviewed by more than just me.

yudai commented 9 years ago

Thank you for merging! :+1:

yudai commented 9 years ago

Btw, this commit https://github.com/zsh-users/zaw/commit/45a411ca0b593d3d67979d3c25efd45fde00e370 changes the default behavior and that can bring up some confusion for most users. If it's not required by other commits, it might be better to revert this one.

willghatch commented 9 years ago

Actually, I saw that it changed the default behavior. It was actually in the section that had merge conflicts (essentially formatting) with something else I merged, so while I was cleaning that up I switched the previous default back.

Thanks for your work. I only merged that first set of commits and not the later ones because I had some issues with the inplace setting and some other stuff you added. However, I like the functionality, and actually added a pull request myself that does some of it (albeit a slightly different way) before I found your code. I didn't merge it yet, in order for the other new maintainer to be able to review it, so if you like the way your patch works better you should make a pull request and we can discuss it.

Anyway, you should definitely make pull requests with any future work you do, and me or the other guy will get stuff merged in.

On Mon, Jun 22, 2015 at 12:59 PM, Iwasaki Yudai notifications@github.com wrote:

Btw, this commit 45a411c https://github.com/zsh-users/zaw/commit/45a411ca0b593d3d67979d3c25efd45fde00e370 changes the default behavior and that can bring up some confusion for most users. If it's not required by other commits, it might be better to revert this one.

— Reply to this email directly or view it on GitHub https://github.com/zsh-users/zaw/pull/61#issuecomment-114221755.

yudai commented 9 years ago

Got it.

As for the inplace search, I feel like my code is a little bit hacky as I was not so familiar with zsh/zaw and wrote it in a "anyway, it looks it's working" way. So if your PR works fine for the same purpose, it would be great to drop my commits and adopt your smarter way :+1:

Here is a video for reference: https://pbs.twimg.com/tweet_video/CA0ufoYVAAAMFbw.mp4

willghatch commented 9 years ago

To clarify -- I added code to allow initial filter contents, primarily for the zaw-history source. I haven't done anything related to in place searching.

yudai commented 9 years ago

Thanks. I realized that your PR is for the initial filter pattern and updated/rebased my code with it.

My inplace search is just use the current shell line as the filter prompt. I'm not sure it makes sense for others. So let me know if you are interested in it. I'll create a PR then.