Simplify FindUtils.parseDollars

[marcelgerber]

I tried to simplify/comment the quite complicated FindReplace helper function parseDollars, but to be honest, I don't know if I made it better or worse.

Let's see if someone understands the function now 😆

The only architectural change is that parseInt is only called after we made sure index is indeed an integer (and not &).

[dangoor]

@njx

[njx]

Marking low priority. (@SAplayer - not intended to be an insult :), but we're starting to try to sort out pull requests that need to be dealt with more urgently from ones that can wait a bit.)

[dangoor]

nit: "unescaped"

[dangoor]

Stealing this from @njx. @SAplayer the change seems fine. Would you be willing to add some test code for this to FindReplace-test.js? I realize there are no tests there now, but it would be nice to have some that demonstrate what this function does (and that it actually does it!)

Thanks!

[njx]

Before merging this, let me check whether it will merge cleanly with the replace-across-files branch - I might have moved the function elsewhere.

[dangoor]

@njx OK. I was surprised it hadn't moved already :)

Getting replace-across-files in is higher priority, so you can just carry on, merge that and then we'll see what needs to happen.

[njx]

Yup, it's moved to FindUtils.js in my branch. Should be easy enough to copy it there after that lands and put up a new PR. (Sorry, I should have landed this before starting the branch in the first place.)

[marcelgerber]

@dangoor There are no tests specificially for this function, but there are many for the functionality:
https://github.com/adobe/brackets/blob/master/test/spec/FindReplace-test.js#L1173-1331

[dangoor]

@SAplayer Ahh, you're right. That's fine.

[marcelgerber]

@dangoor @njx Merged and ready.

[dangoor]

@SAplayer Thanks for merging this in with the recent find changes.