Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Add "replace all" button when doing find and replace #11606

Closed
wants to merge 5 commits into from
Closed

Add "replace all" button when doing find and replace #11606

wants to merge 5 commits into from

Conversation

amrelnaggar
Copy link
Contributor

For #11126.

@zaggino zaggino added this to the Release 1.9 milestone Aug 26, 2016
@ficristo
Copy link
Collaborator

I didn't look closely at the code but I wonder if changing the button id could break extensions.
That said this needs at least a test.

@amrelnaggar
Copy link
Contributor Author

Sorry I've been pretty busy.

I didn't think about extensions. Would it be better if I kept the button id as it was and made a new id for Replace all button? Or unit tests will be enough?

@ficristo
Copy link
Collaborator

@amrelnaggar I looked around on the public available extensions and I think this is safe as is. So only the test is needed.

It is only to understand if this is something that would be contributed back from Adobe: @swmitra @madanbn?

@amrelnaggar
Copy link
Contributor Author

I added the test.

@ficristo
Copy link
Collaborator

@amrelnaggar could you merge master so to resolve the conflicts?

@amrelnaggar
Copy link
Contributor Author

@ficristo All conflicts have been resolved.

If there is anything else needs to be done just let me know.

@ficristo
Copy link
Collaborator

Something went wrong. It seems you merged an old version of master.
You can use git rebase -i ... to drop the last two commits and then try to run git fetch upstream and merge again upstream/master
Alternatively you can open a new fresh PR.

@amrelnaggar
Copy link
Contributor Author

@ficristo I created a new PR #12988.

Closing this PR.

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

Successfully merging this pull request may close these issues.

3 participants