Added new line and tab as replacement in case of regex search

[ficristo]

This should fix #11611.
The idea comes from codemirror/codemirror5#3456 since Brackets do not use the search addon.

[ficristo]

I found on the Notepadd++ repo this file https://github.com/notepad-plus-plus/notepad-plus-plus/blob/35adb1910ba98897ad3a935fa7d712a986289bd3/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp#L53
I don't know anything about the source code of Notepad++ but it seems to have a more complete set of rules for the replacement.
I think for now if would already be good enough to have this PR and, if needed, in future we could add the other rules.
@abose What do you think? Can this be merged?

[FrancescoBorzi]

any news about this?

[rlindner81]

👍 please merge. I ran across this problem multiple times already.

[drewscott]

I'm also waiting on this to be merged.

[abose]

Thanks @ficristo for this change.
So with the change, If someone wants to replace xyz with \n[interpreted as a new line char] it can be done only in regexp mode, and if the replace is performed in a non-regexp mode, \n will be replaced literally[which is the current behavior]. sounds good to me.

can you add unit test cases for this?

[redmunds]

@ficristo This is an awesome improvement!

I think the name parseString() is too generic, though. Maybe regexUnescapeString() ?

Does this need to detect when the backslash itself escaped so that you can (for example) use "\n" to insert "\n" (i.e. not a newline)? Either way, that would be a good unit test.

[ficristo]

While I agree it is a bit generic, I prefer to preserve the name because it matches the one in CodeMirror. But it is not a big deal to change it if you really want.
To insert the chars \ and n in regex mode you need to escape it: \\n.
I'll look at adding some tests.

[abose]

10 find replace integration tests and 2 findinfiles integration tests are failing.
But i guess its failing even without this change for me.
Have to see what change caused the tests to break.

[ficristo]

@abose The failing tests are failing in master too.

[ficristo]

Now with tests. I still have some trouble with \r on FindReplace.

[abose]

@ficristo Are all the cases considered? We could omit the fixes for unit tests that were already failing before this PR. Are there any changes pending for merging this PR?

[ficristo]

@abose I still don't know how to test \r in FindReplace tests.
I tried to add the following code before the test, in the hope to change the new lines, but didn't worked.

runs(function () {
    myDocument.replaceRange(defaultContent.replace(/\n/g, "\r\n"), {line: 0, ch: 0});
    myEditor.centerOnCursor = jasmine.createSpy("centering");
});

Plus I just saw the Search -> Replace All in untitled document. I need to add some test there too?

[ficristo]

I think I've solved the \r issue.

[ficristo]

@abose @redmunds This should be ready to go. Let me know if I need to add more tests.

[ficristo]

Mode to 1.9 because, after all, I think having #10346 first would be better.

[ficristo]

Since this introduce a way to change the line endings I thought it was better to have something like #13152 first.
But given this feature is really demanded @adobe/brackets-committers what do you think of merging this one anyway?

[saurabh95]

I am also in favour in merging #13152, I have tested it myself and code also looks good to me, and I guess the problems mentioned in it(like loosing undo for line endings etc) can be ignored for now.