https://codereview.appspot.com/14565044/diff/1/app/models/charm.js#newcode468
> app/models/charm.js:468: value.sort(function(a, b) {
> This all looks great but I'm curious what this sort method gets you
over the
> standard lexicographic sort. You may also want to add this comment to
the code
> so others know when looking at this.
Thank you for the review and comments, Jeff. I added the following
explanation to the code.
// This sort has several properties that are different than
a
// standard lexicographic sort.
// * Filenames in the root are grouped together, rather than
// sorted along with the names of directories.
// * Sort ignores case, unless case is the only way to
// distinguish between values, in which case it is honored
// per-directory. For example, this is sorted: "a", "b/c",
// "b/d", "B/b", "B/c", "e/f". Notice that "b" directory
values
// and "B" directory values are grouped together, where
they
// would not necessarily be in a simpler case-insensitive
// lexicographic sort.
// If you rip this out for something different and simpler,
that's
// fine; just don't tell me about it. :-) This seemed like
a good
// approach at the time.
On 2013/10/08 21:03:10, jeff.pihach wrote:
> LGTM QA OK with trivial
> https:/ /codereview. appspot. com/14565044/ diff/1/ app/models/ charm.js
> File app/models/charm.js (right):
https:/ /codereview. appspot. com/14565044/ diff/1/ app/models/ charm.js# newcode466 charm.js: 466: setter: function(value) {
> app/models/
> you can quote this instead of commenting it if you like.
https:/ /codereview. appspot. com/14565044/ diff/1/ app/models/ charm.js# newcode468 charm.js: 468: value.sort( function( a, b) {
> app/models/
> This all looks great but I'm curious what this sort method gets you
over the
> standard lexicographic sort. You may also want to add this comment to
the code
> so others know when looking at this.
Thank you for the review and comments, Jeff. I added the following
explanation to the code.
// This sort has several properties that are different than
a
// standard lexicographic sort.
// * Filenames in the root are grouped together, rather than
// sorted along with the names of directories.
// * Sort ignores case, unless case is the only way to
// distinguish between values, in which case it is honored
// per-directory. For example, this is sorted: "a", "b/c",
// "b/d", "B/b", "B/c", "e/f". Notice that "b" directory
values
// and "B" directory values are grouped together, where
they
// would not necessarily be in a simpler case-insensitive
// lexicographic sort.
// If you rip this out for something different and simpler,
that's
// fine; just don't tell me about it. :-) This seemed like
a good
// approach at the time.
https:/ /codereview. appspot. com/14565044/