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

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

Closed
amrelnaggar wants to merge 5 commits into
adobe:masterfrom
amrelnaggar:add-replace-all-button
Closed

Add "replace all" button when doing find and replace#11606
amrelnaggar wants to merge 5 commits into
adobe:masterfrom
amrelnaggar:add-replace-all-button

Conversation

@amrelnaggar
Copy link
Copy Markdown
Contributor

For #11126.

@zaggino zaggino added this to the Release 1.9 milestone Aug 26, 2016
@ficristo
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

I added the test.

@ficristo
Copy link
Copy Markdown
Collaborator

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

@amrelnaggar
Copy link
Copy Markdown
Contributor Author

@ficristo All conflicts have been resolved.

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

@ficristo
Copy link
Copy Markdown
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
Copy Markdown
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