You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR fixes the second issue mentioned in #4409 by comparing the numbers when the filenames are the same for the exception of the number at the end of the string.
I never knew that localeCompare could have more arguments. This works in Chrome, so it will work inside brackets, but will fail once we go to the browser. Maybe is ok to have the previous sorting in FireFox until this is supported by FireFox? I don't think it should take them too long to support it.
I just checked in the debugger tools and using: cmpNames = filename1.toLocaleLowerCase().localeCompare(filename2.toLocaleLowerCase(), undefined, {numeric: true});
Will work better.
@TomMalbran
I know that you may want to keep your implementation for other browsers, but your solution (based on my suggestion) is still not correct. I know it's my mistake to assume that numbers are always suffix to the filenames. You can try out with filenames that have numbers as prefix (eg. 7untitled.js and 12untitled.js), and realize that our solution does not work.
So I think we should switch to use options argument if it is supported by the browser, and keep using the original localeCompare with one argument for browsers like FireFox.
@RaymondLim Actually I prefer to use the options in the localeCompare. Since is already implemented is better to use it. It is also a visual fix, nothing is really broken if we use the old option.
We would still need to use it as I posted it, with undefined as the locale, so that is works for any locale and not just Enlgish. Since JavaScript ditches the extra parameters passed to a function, it works in FireFox passing the options. It just returns the result as if there wasn't options. We can just use he function with the options for every browser.
I totally agree with you to use undefined for locale argument. I used "en" just for a quick testing and not sure that it can take undefined when I posted my comment.
Your changes look good! But we still need two more changes.
It seems like the original issue in [Core][Untitled files][Win only]: Incorrect sort order for untitled files. #4409 was fixed when we switched to localeCompare() and we don't need the if (brackets.platform === "win") { ...} any more. So try it out without it and if you don't see the original issue, then please remove it.
We may need to change the other localeCompare() call with options argument for consistency. This is necessary if the user has custom extensions (like .j7 and .j12), then we need to sort it the same way as we sort by name.
We kind of still need the if (brackets.platform === "win") { ...} since without it Untitled.js will appear after Untitled-1.js. But since we don't add extensions after the Untitled files anymore, this doesn't show when generating them. Is still a problem when files have extensions.
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the second issue mentioned in #4409 by comparing the numbers when the filenames are the same for the exception of the number at the end of the string.
@RaymondLim I was able to get it done today.