[CLOSED] Rename should not include partial file extension in its selection.

[core-ai-bot]

Issue by RaymondLim
Thursday Mar 20, 2014 at 17:42 GMT
Originally opened as adobe/brackets#7265

---

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.

[core-ai-bot]

Comment by TomMalbran
Thursday Mar 20, 2014 at 18:00 GMT

---

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

[core-ai-bot]

Comment by MarcelGerber
Thursday Mar 20, 2014 at 18:45 GMT

---

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

[core-ai-bot]

Comment by RaymondLim
Thursday Mar 20, 2014 at 18:50 GMT

---

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

[core-ai-bot]

Comment by TomMalbran
Thursday Mar 20, 2014 at 18:54 GMT

---

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().

[core-ai-bot]

Comment by MarcelGerber
Thursday Mar 20, 2014 at 19:02 GMT

---

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

[core-ai-bot]

Comment by TomMalbran
Thursday Mar 20, 2014 at 19:33 GMT

---

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.

[core-ai-bot]

Comment by zaggino
Thursday Mar 20, 2014 at 21:03 GMT

---

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.

[core-ai-bot]

Comment by TomMalbran
Thursday Mar 20, 2014 at 22:00 GMT

---

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?

[core-ai-bot]

Comment by zaggino
Thursday Mar 20, 2014 at 22:15 GMT

---

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.

[core-ai-bot]

Comment by peterflynn
Thursday Mar 20, 2014 at 22:30 GMT

---

@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.

[core-ai-bot]

Comment by zaggino
Thursday Mar 20, 2014 at 22:50 GMT

---

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.

[core-ai-bot]

Comment by zaggino
Friday Mar 21, 2014 at 03:36 GMT

---

@peterflynn see adobe/brackets#7242

[core-ai-bot]

Comment by peterflynn
Monday Mar 24, 2014 at 18:39 GMT

---

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

[core-ai-bot]

Comment by redmunds
Monday Mar 24, 2014 at 23:39 GMT

---

FBNC back to@RaymondLim.

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

[core-ai-bot]

Comment by RaymondLim
Monday Mar 24, 2014 at 23:43 GMT

---

Confirmed fixed.