Rename should not include partial file extension in its selection.

[RaymondLim]

1. Have some files with extensions like .html.erb or .css.erb or .config.json.
2. Try to rename one of them in the file tree.

Result: Everything in the filename before the last dot is selected.
Expected: Should select only the filename and exclude everything from the first dot.

[TomMalbran]

Should be fixed by #7242. Or will be fixed for file extensions that Brackets know.

[marcelgerber]

Btw, I'd expect the file tree (on the left) to highlight the same filename part that will be renamed.
So currently for a file like test.html.erb, it looks like this:
test.html.erb
I'd expect:
test.html.erb

@zaggino For #7242

[RaymondLim]

The culprit is getFileExtension() function in FileUtils.js and it is not yet covered in #7242. getFileExtension is using lastIndexOf instead of indexOf.

[TomMalbran]

indexOf doesn't always give the file extension. For example if I have, jquery-1.10.2.min.js, the extension is js no 10.2.min.js. So is not that easy to get the extension.

The way @zaggino is doing it in #7242 might be the best one, maybe it could be moved to getFileExtension().

[marcelgerber]

The problem is that getFileExtension should work for more than the registered languages (I think).
It's not easy indeed.

[TomMalbran]

Yes. We can first try with the registered languages. And when none of them match, use lastIndexOf. We can also optimize the search by having an object with the registered extensions as the keys. Or a RegEx if can take less time.

[zaggino]

Should I rewrite getFileExtension in the PR? We could have an array with all extensions that have a dot inside and loop through them, if not found then use the current behaviour.

[TomMalbran]

I think that we could change that function so that we can fix it for other things, but using an array with extensions can be slow, so we need an alternative if we want to use this solution for things like the files tree. I am not sure how a regexp with all the extensions might work? or if not a an object?

[zaggino]

Object should be fast enough and then just check all parts of the file for match. I think that creating a regExp dynamically would be overly complicated. With object we'd have usually just 1-2 checks.

[peterflynn]

@zaggino @TomMalbran Since FileUtils.getFileExtension() is so widely used, I think it's best to keep its behavior simple for now. Especially since we don't know what extensions are using it for and what assumptions they may have about how it's going to behave, etc... I think it's ok to have the "smart" file extension stuff only be used by the rename code for now. It's still easy to generalize more later if we find that most other use cases want the same.

[zaggino]

What about creating FileUtils.getSmartFileExtension() ? That way even extensions who choose so can benefit from it and we can easily switch (if we ever decide to) later.

[zaggino]

@peterflynn see #7242

[peterflynn]

Reviewed - low priority @zaggino since PR with fix already in progress

[redmunds]

FBNC back to @RaymondLim.

The original bug is fixed, and also the issue mentioned in this comment by @SAplayer is also fixed.

[RaymondLim]

Confirmed fixed.