Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documents are sorted by a number that appears anywhere in their name #296

Open
tommi-h opened this issue Oct 8, 2021 · 2 comments
Open
Labels
bug Something isn't working

Comments

@tommi-h
Copy link

tommi-h commented Oct 8, 2021

Expected Behavior

Documents are sorted alphabetically in the "Pages in X" section and the "View All Docs" page.

Actual Behavior

If a document has a number (digit) anywhere in its name, that number is used as the sort key, instead of the name.

To Reproduce

Add two documents to a folder in Drive. Name the documents as, for example, "Background Data" and "Status Report 06/2020". Now the latter document is sorted before the former, which is counter-intuitive.

Additional Information

I was unable to tell whether this is a bug or intended behaviour. I did not find any tests for this.

Possible Solution

This behaviour seems to be implemented in function determineSort in server/list.js. The regular expression catches a digit anywhere in the name. However, I do not see the need to use any regex here. Documents could be simply sorted by their name. If a user wants to control the sorting of the documents as described in Library demo site (by prepending the document names with numbers), that works "out of the box" in string sorting, without any special handling.

@tommi-h tommi-h added the bug Something isn't working label Oct 8, 2021
@afischer
Copy link
Member

afischer commented Oct 8, 2021

Thank you for the report!

This is definitely not our intended behavior — The purpose of the non-standard sorting is to allow users to manually order folders and files by adding a #. or #) to the beginning of the file. We then trim those leading numbers (as well as trailing tags) in the cleanName function:

library/server/docs.js

Lines 16 to 21 in 63e8a1b

return name
.trim()
// eslint-disable-next-line no-useless-escape
.replace(/^(\d+[-–—_\s]*)([^\d\/\-^\s]+)/, '$2') // remove leading numbers and delimiters
.replace(/\s*\|\s*([^|]+)$/i, '') // remove trailing pipe and tags
.replace(/\.[^.]+$/, '') // remove file extensions

We should definitely tighten up this sorting logic, and maybe document the leading number trimming logic to show which delimiters are valid for adjusting sorting.

@alipman88
Copy link

However, I do not see the need to use any regex here. Documents could be simply sorted by their name. If a user wants to control the sorting of the documents as described in Library demo site (by prepending the document names with numbers), that works "out of the box" in string sorting, without any special handling.

I believe @tommi-h is correct here: there's no need for determineSort function. A string beginning with 01 - will always come before one beginning with 02 - when sorted alphanumerically. No need for a function/regex to extract the leading numeric characters.

Unless the eventual goal is to parse and sort integers (so that 2 will come before 11, even without zero-padding), just drop determineSort:

diff --git a/server/list.js b/server/list.js
index d4a30af..d14fb37 100644
--- a/server/list.js
+++ b/server/list.js
@@ -184,7 +184,7 @@ function produceTree(files, firstParent) {
       prettyName,
       tags,
       resourceType: cleanResourceType(mimeType),
-      sort: determineSort(name),
+      sort: name,
       slug,
       isTrashCan: slug === 'trash' && parents.includes(driveId)
     })
@@ -257,7 +257,7 @@ function buildTreeFromData(rootParent, previousData, breadcrumb) {
     homePrettyName,
     id: rootParent,
     breadcrumb,
-    sort: parentInfo ? determineSort(parentInfo.name) : Infinity // some number here that could be used to sort later
+    sort: parentInfo ? parentInfo.name : Infinity // some number here that could be used to sort later
   }

   // detect redirects or purge cache for items not contained in trash
@@ -373,13 +373,6 @@ function handleUpdates(id, {info: lastInfo, tree: lastTree}) {
   })
 }

-function determineSort(name = '') {
-  const sort = name.match(/(\d+)[^\d]/)
-  // to be consistent with drive API, we will do string sort
-  // also means we can sort off a single field when number is absent
-  return sort ? sort[1] : name // items without sort go alphabetically
-}
-
 function cleanResourceType(mimeType) {
   const match = mimeType.match(/application\/vnd.google-apps.(.+)$/)
   if (!match) return mimeType

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants