bug#60601: 29.0.60; project-find-files reports files in seemingly random order

bug#60601: 29.0.60; project-find-files reports files in seemingly random order

[Jostein Kjønigsen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 10040 bytes --]

When working in large projects, and using completion engines like Helm, 
one often navigates within an Emacs-project by first typing some 
preliminary filtering-criteria, observing the result, and then further 
limiting the results until one eventually finds the one file you want to 
work with.

For this flow to work well, one needs to have related files show up 
close to one another, and not being listed in "random" order.

However, in some projects I've observed that generated files (not part 
of VC) are shown on top, and VC-files comes later.

When the generated files are supposed to be alongside the checked in 
files, this makes navigation using project-find-file hard, because of 
the low data-locality in the presented results.

Example:

- src/someFile.d.ts   # compiled files
- src/someFile.js
- tests/someTest.d.ts
- tests/someTest.js
- src/someFile.ts     # vc-files
- tests/someTest.ts

If the results are sorted, the proper files are much easier to find quickly:

- src/someFile.d.ts
- src/someFile.js
- src/someFile.ts
- tests/someTest.d.ts
- tests/someTest.js
- tests/someTest.ts

Are there any reason we're currently not doing that?

If we can agree sorting the result is a proper thing to do, attached is 
a patch which implements this change.

--
Jostein

In GNU Emacs 29.0.60 (build 13, x86_64-pc-linux-gnu, GTK+ Version

  3.24.33, cairo version 1.16.0) of 2023-01-06 built on dev-jostein
Repository revision: 699711786ac7aa6d6524467076a45568e283e061
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12201001
System Description: Ubuntu 22.04.1 LTS

Configured using:
  'configure --with-json --with-tree-sitter
  PKG_CONFIG_PATH=/home/jostein/.opam/default/lib/pkgconfig:'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LC_MONETARY: nb_NO.UTF-8
   value of $LC_NUMERIC: nb_NO.UTF-8
   value of $LC_TIME: nb_NO.UTF-8
   value of $LANG: en_US.UTF-8
   locale-coding-system: utf-8-unix

Major mode: ELisp/l

Minor modes in effect:
   global-git-commit-mode: t
   magit-auto-revert-mode: t
   bug-reference-prog-mode: t
   elisp-slime-nav-mode: t
   paredit-mode: t
   highlight-symbol-mode: t
   flycheck-mode: t
   editorconfig-mode: t
   company-mode: t
   which-function-mode: t
   helm-mode: t
   helm-minibuffer-history-mode: t
   shell-dirtrack-mode: t
   helm--remap-mouse-mode: t
   async-bytecomp-package-mode: t
   delete-selection-mode: t
   global-auto-revert-mode: t
   yas-global-mode: t
   yas-minor-mode: t
   global-nlinum-mode: t
   nlinum-mode: t
   ido-yes-or-no-mode: t
   override-global-mode: t
   server-mode: t
   global-hl-line-mode: t
   pixel-scroll-precision-mode: t
   doom-modeline-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   column-number-mode: t
   line-number-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   hs-minor-mode: t

Load-path shadows:
/home/jostein/.emacs.d/elpa/transient-20221202.1727/transient hides 
/home/jostein/build/emacs/lisp/transient

Features:
(shadow sort emacsbug flyspell ispell mail-extr pulse cl-print dired-aux
misearch multi-isearch tabify cus-start help-fns radix-tree mule-util
apropos helm-command helm-elisp helm-eval edebug helm-info executable
markdown-mode color elec-pair typescript-ts-mode js c-ts-mode cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs treesit goto-addr magit-extras face-remap magit-submodule
magit-obsolete magit-blame magit-stash magit-reflog magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode diff
git-commit log-edit message sendmail yank-media rfc822 mml mml-sec epa
derived epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader
pcvs-util magit-core magit-autorevert magit-margin magit-transient
magit-process with-editor magit-mode transient magit-git magit-base
magit-section crm compat-27 compat-26 disp-table bug-reference vc-git
diff-mode vc-dispatcher vc-svn winner ffap tramp-archive tramp-gvfs
tramp-cache time-stamp zeroconf dbus add-log ido-completing-read+
memoize minibuf-eldef elisp-slime-nav paredit highlight-symbol flycheck
editorconfig editorconfig-core editorconfig-core-handle
editorconfig-fnmatch company-oddmuse company-keywords company-etags
etags fileloop generator company-gtags company-dabbrev-code
company-dabbrev company-files company-clang company-capf company-cmake
company-semantic company-template company-bbdb company eglot
external-completion array jsonrpc ert ewoc debug backtrace flymake-proc
flymake warnings which-func hideshow eww url-queue thingatpt shr
pixel-fill kinsoku url-file svg xml dom puny mm-url gnus nnheader
gnus-util mail-utils range mm-util mail-prsvr helm-imenu pcase imenu
ob-plantuml org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-src ob-comint org-pcomplete org-list org-footnote org-faces
org-entities noutline outline ob-emacs-lisp ob-core ob-eval org-cycle
org-table ol org-fold org-fold-core org-keys oc org-loaddefs find-func
cal-menu calendar cal-loaddefs org-version org-compat org-macs helm-mode
helm-misc helm-files image-dired image-dired-tags image-dired-external
image-dired-util xdg image-mode dired dired-loaddefs exif tramp
tramp-loaddefs trampver tramp-integration cus-edit pp cus-load icons
wid-edit files-x tramp-compat shell pcomplete parse-time iso8601
time-date ls-lisp helm-buffers helm-occur helm-tags helm-locate
helm-grep helm-regexp format-spec helm-utils helm-help helm-types helm
helm-global-bindings helm-easymenu helm-core async-bytecomp helm-source
helm-multi-match helm-lib async helm-config delsel autorevert filenotify
yasnippet nlinum linum ido-yes-or-no advice ido edmacro kmacro
use-package-bind-key bind-key easy-mmode xref project server hl-line
pixel-scroll cua-base compile-eslint compile comint ansi-osc ansi-color
ring doom-modeline doom-modeline-segments doom-modeline-env
doom-modeline-core all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons shrink-path rx f f-shortdoc shortdoc
text-property-search s dash compat compat-macs dracula-theme cl-extra
help-mode use-package-ensure use-package-core finder-inf
expand-region-autoloads ido-yes-or-no-autoloads doom-modeline-autoloads
elisp-slime-nav-autoloads multiple-cursors-autoloads
dracula-theme-autoloads crontab-mode-autoloads
highlight-symbol-autoloads ssh-config-mode-autoloads dap-mode-autoloads
lsp-treemacs-autoloads treemacs-autoloads cfrs-autoloads
posframe-autoloads powershell-autoloads company-web-autoloads
shrink-path-autoloads paredit-autoloads bui-autoloads
all-the-icons-autoloads rust-mode-autoloads yaml-mode-autoloads
helm-autoloads popup-autoloads helm-core-autoloads async-autoloads
lsp-docker-autoloads yaml-autoloads lsp-mode-autoloads spinner-autoloads
f-autoloads hydra-autoloads nlinum-autoloads ht-autoloads
undo-tree-autoloads queue-autoloads ace-window-autoloads avy-autoloads
cmake-mode-autoloads bmx-mode-autoloads s-autoloads company-autoloads
ido-completing-read+-autoloads memoize-autoloads lv-autoloads
magit-autoloads magit-section-autoloads git-commit-autoloads
with-editor-autoloads transient-autoloads compat-autoloads
editorconfig-autoloads yasnippet-autoloads web-completion-data-autoloads
cargo-autoloads markdown-mode-autoloads pfuture-autoloads
flycheck-autoloads pkg-info-autoloads epl-autoloads info dash-autoloads
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie generate-lisp-file url-domsuf url-util mailcap
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
password-cache json subr-x map byte-opt gv bytecomp byte-compile
url-vars cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 663746 79685)
  (symbols 48 45967 1)
  (strings 32 201584 10115)
  (string-bytes 1 6033340)
  (vectors 16 85585)
  (vector-slots 8 1060389 121544)
  (floats 8 989 979)
  (intervals 56 9805 2324)
  (buffers 984 57))

-- 
Kind regards
*Jostein Kjønigsen*
Software Architect
SuperOffice AS
www.superoffice.com <https://www.superoffice.com>

[-- Attachment #1.2: Type: text/html, Size: 14797 bytes --]

[-- Attachment #2: 0001-Make-project-find-files-return-files-in-sorted-order.patch --]
[-- Type: text/x-patch, Size: 1332 bytes --]

From 6df088f98e7c4e7cd3c00919a5534fd315d2cde1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jostein=20Kj=C3=B8nigsen?= <jostein@kjonigsen.net>
Date: Fri, 6 Jan 2023 10:33:51 +0100
Subject: [PATCH] Make project-find-files return files in sorted order.

---
 lisp/progmodes/project.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 730998727ce..03119b16ce7 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1080,11 +1080,11 @@ project-find-file-in
                              (concat dir "/"))
                            vc-directory-exclusion-list))
          (all-files
-          (if include-all
-              (mapcan
-               (lambda (dir) (project--files-in-directory dir vc-dirs-ignores))
-               dirs)
-            (project-files project dirs)))
+          (sort (if include-all
+                    (mapcan
+                     (lambda (dir) (project--files-in-directory dir vc-dirs-ignores))
+                     dirs)
+                  (project-files project dirs)) #'string<))
          (completion-ignore-case read-file-name-completion-ignore-case)
          (file (funcall project-read-file-name-function
                         "Find file" all-files nil 'file-name-history
-- 
2.34.1

bug#60601: 29.0.60; project-find-files reports files in seemingly random order

[Dmitry Gutov]

Hi!

On 06/01/2023 11:48, Jostein Kjønigsen via Bug reports for GNU Emacs, 
the Swiss army knife of text editors wrote:
> When working in large projects, and using completion engines like Helm, 
> one often navigates within an Emacs-project by first typing some 
> preliminary filtering-criteria, observing the result, and then further 
> limiting the results until one eventually finds the one file you want to 
> work with.
> 
> For this flow to work well, one needs to have related files show up 
> close to one another, and not being listed in "random" order.

One of the problems is, Helm is not a proper/compatible completing-read 
substitute. Just like Ivy, which I myself use.

If you check out the default completion, you'll see the file names are 
sorted, because a completing-read-function is supposed to obey the 
display-sort-function metadata, and when it's absent -- the variable 
completions-sort (in Emacs <29 it was hardcoded alphabetical sort by 
default).

So one solution would be to use a framework-specific way to apply 
sorting, e.g. helm-completing-read-handlers-alist. In Ivy's case, I 
think ivy-sort-functions-alist is supposed to fill that role, but 
apparently it's not used when the collection is a function. I'll 
probably file that as a bug later.

> However, in some projects I've observed that generated files (not part 
> of VC) are shown on top, and VC-files comes later.
> 
> When the generated files are supposed to be alongside the checked in 
> files, this makes navigation using project-find-file hard, because of 
> the low data-locality in the presented results.

This never bothered me much personally, but the reason is apparently 
that 'git ls-files' behaves like that. We do apply sorting to find's 
output (the default project-files impl), starting with 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=23179#296.

> Example:
> 
> - src/someFile.d.ts   # compiled files
> - src/someFile.js
> - tests/someTest.d.ts
> - tests/someTest.js
> - src/someFile.ts     # vc-files
> - tests/someTest.ts
> 
> If the results are sorted, the proper files are much easier to find quickly:
> 
> - src/someFile.d.ts
> - src/someFile.js
> - src/someFile.ts
> - tests/someTest.d.ts
> - tests/someTest.js
> - tests/someTest.ts
> 
> Are there any reason we're currently not doing that?
> 
> If we can agree sorting the result is a proper thing to do, attached is 
> a patch which implements this change.

No hard reason not to, but the minor ones are:

- A little extra garbage collection.
- We'll probably apply the new sorting in project-files, keeping the one 
in project--files-in-directory already in place. Then I suppose we'll 
need to document the new restriction on project-files that it returns 
alphabetically sorted files (option A)?

Or if we move the sorting to project-find-file-in, that will keep the 
xref output unsorted. Do we sort the results after the search is done 
instead? That might be an improvement, but it could create a performance 
hit in the pathological case when there are a lot more matches than files.

So we can do that (option A), probably.

Alternatively, this could be a good occasion to file the bug reports to 
Helm and Ivy, to bring their completing-read functions closer to 
conformant behavior.