Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Replace #374

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Implement Replace #374

wants to merge 4 commits into from

Conversation

pdl
Copy link
Contributor

@pdl pdl commented Nov 29, 2016

This is a simple implementation of a replace feature.
Find continues to work as before, but there is an additional replace line and button. Pressing the button, or hitting enter with the replace line focussed, does a replace.
No attempt is made to perform capture interpolation, e.g. of$1 (It's not hard, and I have all the necessary bits of code, but different editors have different behaviours so I want to separate out that bikeshedding specification discussion).

One issue I have found is that the tab order is not stable. I don't know if I'm doing something wrong or whether this is a (mis)feature of the way blessed objects are created, where they specify their parents rather than being actively appended to them as you would with a DOM. It is usable, and always possible to tab to all the fields, but sometimes when hitting tab from the find field, you get the replace field, as you'd expect, and other times, you get the Replace button. I'm not able to solve this but I wonder if the react-blessed rewrite will take care of it.

Note that this renders #367 redundant.

@dbkaplun
Copy link
Member

dbkaplun commented Dec 23, 2016

Looks great! Only thing is the replace doesn't work for me. :) Here's what I tried:

  • ./slap.js package.json from slap project root
  • Ctrl+F, slap, Tab, test (at this point the Find line says "slap" and the replace line says "test")
  • Enter

At which point I'm not sure why but nothing happens.

@pdl
Copy link
Contributor Author

pdl commented Dec 23, 2016

OK, looks like the enter key is not triggering the button - you can tab into the Replace button and trigger it manually. I'll look into it.

@pdl
Copy link
Contributor Author

pdl commented Dec 23, 2016

OK, that should be fixed. I've also added a commit which implements replacement text as after a bit more investigation I'm confident in the behaviour. This adds a dependency, which I've written for the purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants