[PATCH] Allow hiding stashes from vc-dir

[PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

[-- Attachment #1: Type: text/plain, Size: 237 bytes --]

For various reasons I tend to have a lot of stashes hanging around,
which magit can autohide for you. vc-dir doesnʼt have such an option,
and has no commands for doing things with stashes anyway, so I came up
with the following. 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vc-git-allow-not-showing-git-stash-list-in-vc-dir.patch --]
[-- Type: text/x-patch, Size: 4416 bytes --]

From b02f05fc569f3946366ac2ede27349cd09618265 Mon Sep 17 00:00:00 2001
From: Robert Pluim <rpluim@gmail.com>
Date: Tue, 5 Feb 2019 10:51:42 +0100
Subject: [PATCH] vc-git: allow not showing git stash list in vc-dir
To: emacs-devel@gnu.org

* lisp/vc/vc-git.el (vc-git-show-stash): New variable.
(vc-git-dir-extra-headers): Only show stash list when
vc-git-show-stash is t.
---
 etc/NEWS          |  3 +++
 lisp/vc/vc-git.el | 49 ++++++++++++++++++++++++++---------------------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 271cc6e044..8c3993474c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -733,6 +733,9 @@ The default value is 'find-dired-sort-by-filename'.
 *** New command 'log-edit-generate-changelog-from-diff', bound to C-c C-w.
 This generates ChangeLog entries from the VC fileset diff.
 
+*** New customizable variable 'vc-git-show-stash'.
+Default t, set it to nil to hide stashes in 'vc-dir' when using git.
+
 *** Recording ChangeLog entries doesn't require an actual file.
 If a ChangeLog file doesn't exist, and if the new variable
 'add-log-dont-create-changelog-file' is non-nil (which is the
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 9715aea1fd..615474cd76 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -192,6 +192,11 @@ vc-git-grep-template
   :type 'string
   :version "27.1")
 
+(defcustom vc-git-show-stash t
+  "Whether to show the git stash list."
+  :type 'boolean
+  :version "27.1")
+
 ;; History of Git commands.
 (defvar vc-git-history nil)
 
@@ -655,7 +660,7 @@ vc-git-dir-extra-headers
   (let ((str (with-output-to-string
                (with-current-buffer standard-output
                  (vc-git--out-ok "symbolic-ref" "HEAD"))))
-	(stash (vc-git-stash-list))
+	(stash (when vc-git-show-stash (vc-git-stash-list)))
 	(stash-help-echo "Use M-x vc-git-stash to create stashes.")
 	branch remote remote-url)
     (if (string-match "^\\(refs/heads/\\)?\\(.+\\)$" str)
@@ -688,30 +693,30 @@ vc-git-dir-extra-headers
 	(propertize "Remote     : " 'face 'font-lock-type-face)
 	(propertize remote-url
 		    'face 'font-lock-variable-name-face)))
-     "\n"
      ;; For now just a heading, key bindings can be added later for various bisect actions
      (when (file-exists-p (expand-file-name ".git/BISECT_START" (vc-git-root dir)))
-       (propertize  "Bisect     : in progress\n" 'face 'font-lock-warning-face))
+       (propertize  "\nBisect     : in progress" 'face 'font-lock-warning-face))
      (when (file-exists-p (expand-file-name ".git/rebase-apply" (vc-git-root dir)))
-       (propertize  "Rebase     : in progress\n" 'face 'font-lock-warning-face))
-     (if stash
-       (concat
-	(propertize "Stash      :\n" 'face 'font-lock-type-face
-		    'help-echo stash-help-echo)
-	(mapconcat
-	 (lambda (x)
-	   (propertize x
-		       'face 'font-lock-variable-name-face
-		       'mouse-face 'highlight
-		       'help-echo "mouse-3: Show stash menu\nRET: Show stash\nA: Apply stash\nP: Apply and remove stash (pop)\nC-k: Delete stash"
-		       'keymap vc-git-stash-map))
-	 stash "\n"))
-       (concat
-	(propertize "Stash      : " 'face 'font-lock-type-face
-		    'help-echo stash-help-echo)
-	(propertize "Nothing stashed"
-		    'help-echo stash-help-echo
-		    'face 'font-lock-variable-name-face))))))
+       (propertize  "\nRebase     : in progress" 'face 'font-lock-warning-face))
+     (when vc-git-show-stash
+       (if stash
+           (concat
+            (propertize "\nStash      :\n" 'face 'font-lock-type-face
+                        'help-echo stash-help-echo)
+            (mapconcat
+             (lambda (x)
+               (propertize x
+                           'face 'font-lock-variable-name-face
+                           'mouse-face 'highlight
+                           'help-echo "mouse-3: Show stash menu\nRET: Show stash\nA: Apply stash\nP: Apply and remove stash (pop)\nC-k: Delete stash"
+                           'keymap vc-git-stash-map))
+             stash "\n"))
+         (concat
+          (propertize "\nStash      : " 'face 'font-lock-type-face
+                      'help-echo stash-help-echo)
+          (propertize "Nothing stashed"
+                      'help-echo stash-help-echo
+                      'face 'font-lock-variable-name-face)))))))
 
 (defun vc-git-branches ()
   "Return the existing branches, as a list of strings.
-- 
2.23.0

Re: [PATCH] Allow hiding stashes from vc-dir

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> For various reasons I tend to have a lot of stashes hanging around,
> which magit can autohide for you. vc-dir doesnʼt have such an option,
> and has no commands for doing things with stashes anyway, so I came up
> with the following. 

There's a bunch of commands for stashes in vc-dir -- `P' and `C-k', for
instance.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [PATCH] Allow hiding stashes from vc-dir

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Tue, 15 Oct 2019 14:48:28 +0200
> 
> +*** New customizable variable 'vc-git-show-stash'.
> +Default t, set it to nil to hide stashes in 'vc-dir' when using git.

How long is your stash list?  A list that is too long can clutter the
vc-dir display, no?  So maybe turn it off by default, or show only the
first few stashes by default?

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Tue, 15 Oct 2019 16:06:20 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Tue, 15 Oct 2019 14:48:28 +0200
    >> 
    >> +*** New customizable variable 'vc-git-show-stash'.
    >> +Default t, set it to nil to hide stashes in 'vc-dir' when using git.

    Eli> How long is your stash list?  A list that is too long can clutter the
    Eli> vc-dir display, no?  So maybe turn it off by default, or show only the
    Eli> first few stashes by default?

I currently have 45 stashes in my emacs repo. Note that the stashes
are currently all shown by default, this option allows me to hide them
(I guess that means my NEWS entry was unclear).

Robert

Re: [PATCH] Allow hiding stashes from vc-dir

[Michael Albinus]

Robert Pluim <rpluim@gmail.com> writes:

> +*** New customizable variable 'vc-git-show-stash'.

In Emacs, they are called "user option".

Saying this, I found several entries of "customizable variable" in
etc/NEWS. And also other entries of "user option".

Shall be harmonized, I believe.

Best regards, Michael.

Re: [PATCH] Allow hiding stashes from vc-dir

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 15 Oct 2019 16:02:59 +0200
> 
>     >> +*** New customizable variable 'vc-git-show-stash'.
>     >> +Default t, set it to nil to hide stashes in 'vc-dir' when using git.
> 
>     Eli> How long is your stash list?  A list that is too long can clutter the
>     Eli> vc-dir display, no?  So maybe turn it off by default, or show only the
>     Eli> first few stashes by default?
> 
> I currently have 45 stashes in my emacs repo. Note that the stashes
> are currently all shown by default, this option allows me to hide them

Ah, then apologies for my misunderstanding.

> (I guess that means my NEWS entry was unclear).

It says that its default, t, will NOT hide the stashes.  the name of
the option also says that.  I think you want to rephrase in order to
make the intent of the option more clear: say that it allows to hide
the stashes.

Re: [PATCH] Allow hiding stashes from vc-dir

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Tue, 15 Oct 2019 16:40:44 +0200
> 
> Robert Pluim <rpluim@gmail.com> writes:
> 
> > +*** New customizable variable 'vc-git-show-stash'.
> 
> In Emacs, they are called "user option".
> 
> Saying this, I found several entries of "customizable variable" in
> etc/NEWS. And also other entries of "user option".
> 
> Shall be harmonized, I believe.

Yes, "user option" is better.

Re: [PATCH] Allow hiding stashes from vc-dir

[Juri Linkov]

>> +*** New customizable variable 'vc-git-show-stash'.
>> +Default t, set it to nil to hide stashes in 'vc-dir' when using git.
>
> How long is your stash list?  A list that is too long can clutter the
> vc-dir display, no?  So maybe turn it off by default, or show only the
> first few stashes by default?

This would be the best option: in case of the number of stashes more
than some limit maybe defined by the numeric value of 'vc-git-show-stash'
with the default value of 1, add the toggle button "[Show all stashes]".

Or without adding a new option, just display that button always,
like the button at the end of vc-log "[Show unlimited entries]".

Re: [PATCH] Allow hiding stashes from vc-dir

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> This would be the best option: in case of the number of stashes more
> than some limit maybe defined by the numeric value of 'vc-git-show-stash'
> with the default value of 1, add the toggle button "[Show all stashes]".

Yes, that'd be great.  I periodically have a whole bunch of stashes in
my Emacs repo, and toggling the list interactively would be the best
solution, I think.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Wed, 16 Oct 2019 03:28:07 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Juri Linkov <juri@linkov.net> writes:
    >> This would be the best option: in case of the number of stashes more
    >> than some limit maybe defined by the numeric value of 'vc-git-show-stash'
    >> with the default value of 1, add the toggle button "[Show all stashes]".

    Lars> Yes, that'd be great.  I periodically have a whole bunch of stashes in
    Lars> my Emacs repo, and toggling the list interactively would be the best
    Lars> solution, I think.

OK, Iʼll rework the patch a bit. Iʼve also discovered that vc-dir has
commands for manipulating stashes, so limiting it to eg 5 would help
me (those commands are only bound when youʼre in the stash list, which
is why I never noticed them).

Robert

Re: [PATCH] Allow hiding stashes from vc-dir

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> Saying this, I found several entries of "customizable variable" in
>> etc/NEWS. And also other entries of "user option".
>>
>> Shall be harmonized, I believe.
>
> Yes, "user option" is better.

Done.

Best regards, Michael.

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

>>>>> On Wed, 16 Oct 2019 10:57:18 +0200, Robert Pluim <rpluim@gmail.com> said:

>>>>> On Wed, 16 Oct 2019 03:28:07 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Yes, that'd be great.  I periodically have a whole bunch of stashes in
    Lars> my Emacs repo, and toggling the list interactively would be the best
    Lars> solution, I think.

    Robert> OK, Iʼll rework the patch a bit. Iʼve also discovered that vc-dir has
    Robert> commands for manipulating stashes, so limiting it to eg 5 would help
    Robert> me (those commands are only bound when youʼre in the stash list, which
    Robert> is why I never noticed them).

Toggling with a configurable limit implemented in attached. I did feel
like I was reinventing the wheel a bit, is there no "create a bunch of
strings/regions and apply properties to them" library?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-button-to-vc-dir-to-toggle-visibility-of-stash-l.patch --]
[-- Type: text/x-patch, Size: 8192 bytes --]

From 3197794363e16a617db86de32912e7a1085364e7 Mon Sep 17 00:00:00 2001
From: Robert Pluim <rpluim@gmail.com>
Date: Thu, 17 Oct 2019 15:55:06 +0200
Subject: [PATCH] Add button to vc-dir to toggle visibility of stash list
To: emacs-devel@gnu.org

* lisp/vc/vc-git.el (vc-git-show-stash): New user option.
(vc-git-make-stash-button): Create button that allows hiding the stash
list.
(vc-git-dir-extra-headers): Split stash list into hideable and
non-hideable parts depending on value of vc-git-show-stash.  Add
button to toggle visibility of hideable part.

* etc/NEWS: Announce it.
---
 etc/NEWS          |   5 +++
 lisp/vc/vc-git.el | 102 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 90 insertions(+), 17 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index e1eb74f86e..9126b007ea 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -810,6 +810,11 @@ The default value is 'find-dired-sort-by-filename'.
 *** New command 'log-edit-generate-changelog-from-diff', bound to 'C-c C-w'.
 This generates ChangeLog entries from the VC fileset diff.
 
+*** 'vc-dir' now shows a button allowing you to hide the stash list.
+Controlled by user option 'vc-git-show-stash'.  Default t means show
+the entire list as before.  An integer value limits the list length
+(but still allows you to show the entire list via the button).
+
 *** Recording ChangeLog entries doesn't require an actual file.
 If a ChangeLog file doesn't exist, and if the new user option
 'add-log-dont-create-changelog-file' is non-nil (which is the
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 9715aea1fd..1976aca1c9 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -108,6 +108,8 @@
   (require 'vc)
   (require 'vc-dir))
 
+(declare-function cl-subseq "cl-extra" (seq start &optional end))
+
 (defgroup vc-git nil
   "VC Git backend."
   :version "24.1"
@@ -192,6 +194,21 @@ vc-git-grep-template
   :type 'string
   :version "27.1")
 
+(defcustom vc-git-show-stash t
+  "How much of the git stash list to show by default.
+Default t means all, otherwise an integer specifying the maximum
+number to show.  A text button is always shown allowing you to
+toggle display of the entire list."
+  :type '(choice (const :tag "All" t)
+                 (integer :tag "Limit"
+                          :validate
+                          (lambda (widget)
+                            (unless (>= (widget-value widget) 0)
+                              (widget-put widget :error
+                                          "Invalid value: must be a non-negative integer")
+                              widget))))
+  :version "27.1")
+
 ;; History of Git commands.
 (defvar vc-git-history nil)
 
@@ -635,6 +652,18 @@ vc-git-stash-map
     (define-key map "S" 'vc-git-stash-snapshot)
     map))
 
+(defun vc-git-make-stash-button ()
+    (make-text-button "Show/hide stashes" nil
+                      'action
+                      (lambda (&rest _ignore)
+                          (let* ((inhibit-read-only t)
+                                 (start (next-single-property-change (point-min) 'vc-git-hideable))
+                                 (end (next-single-property-change start 'vc-git-hideable)))
+                            (add-text-properties
+                             start end
+                             `(invisible ,(not (get-text-property start 'invisible))))))
+                      'help-echo "mouse-2, RET: Show/hide stashes"))
+
 (defvar vc-git-stash-menu-map
   (let ((map (make-sparse-keymap "Git Stash")))
     (define-key map [de]
@@ -655,9 +684,11 @@ vc-git-dir-extra-headers
   (let ((str (with-output-to-string
                (with-current-buffer standard-output
                  (vc-git--out-ok "symbolic-ref" "HEAD"))))
-	(stash (vc-git-stash-list))
+	(stash-list (vc-git-stash-list))
 	(stash-help-echo "Use M-x vc-git-stash to create stashes.")
-	branch remote remote-url)
+        (stash-list-help-echo "mouse-3: Show stash menu\nRET: Show stash\nA: Apply stash\nP: Apply and remove stash (pop)\nC-k: Delete stash")
+
+	branch remote remote-url stash-button stash-string)
     (if (string-match "^\\(refs/heads/\\)?\\(.+\\)$" str)
 	(progn
 	  (setq branch (match-string 2 str))
@@ -677,6 +708,50 @@ vc-git-dir-extra-headers
 	  (when (string-match "\\([^\n]+\\)" remote-url)
 	    (setq remote-url (match-string 1 remote-url))))
       (setq branch "not (detached HEAD)"))
+    (when stash-list
+      (let* ((limit
+              (if (integerp vc-git-show-stash)
+                  (min vc-git-show-stash (length stash-list))
+                (length stash-list)))
+             (shown-stashes (cl-subseq stash-list 0 limit))
+             (hidden-stashes (cl-subseq stash-list limit))
+             (all-hideable (eq vc-git-show-stash t)))
+        (setq stash-button (vc-git-make-stash-button)
+              stash-string
+              (concat
+               (when shown-stashes
+                 (concat
+                  (propertize "\n"
+                              'vc-git-hideable all-hideable)
+                  (mapconcat
+                   (lambda (x)
+                     (propertize x
+                                 'face 'font-lock-variable-name-face
+                                 'mouse-face 'highlight
+                                 'vc-git-hideable all-hideable
+                                 'help-echo stash-list-help-echo
+                                 'keymap vc-git-stash-map))
+                   shown-stashes
+                   (propertize "\n"
+                               'vc-git-hideable all-hideable))))
+               (when hidden-stashes
+                 (concat
+                  (propertize "\n"
+                              'invisible t
+                              'vc-git-hideable t)
+                  (mapconcat
+                   (lambda (x)
+                     (propertize x
+                                 'face 'font-lock-variable-name-face
+                                 'mouse-face 'highlight
+                                 'invisible t
+                                 'vc-git-hideable t
+                                 'help-echo stash-list-help-echo
+                                 'keymap vc-git-stash-map))
+                   hidden-stashes
+                   (propertize "\n"
+                               'invisible t
+                               'vc-git-hideable t))))))))
     ;; FIXME: maybe use a different face when nothing is stashed.
     (concat
      (propertize "Branch     : " 'face 'font-lock-type-face)
@@ -688,26 +763,19 @@ vc-git-dir-extra-headers
 	(propertize "Remote     : " 'face 'font-lock-type-face)
 	(propertize remote-url
 		    'face 'font-lock-variable-name-face)))
-     "\n"
      ;; For now just a heading, key bindings can be added later for various bisect actions
      (when (file-exists-p (expand-file-name ".git/BISECT_START" (vc-git-root dir)))
-       (propertize  "Bisect     : in progress\n" 'face 'font-lock-warning-face))
+       (propertize  "\nBisect     : in progress" 'face 'font-lock-warning-face))
      (when (file-exists-p (expand-file-name ".git/rebase-apply" (vc-git-root dir)))
-       (propertize  "Rebase     : in progress\n" 'face 'font-lock-warning-face))
-     (if stash
+       (propertize  "\nRebase     : in progress" 'face 'font-lock-warning-face))
+     (if stash-list
        (concat
-	(propertize "Stash      :\n" 'face 'font-lock-type-face
-		    'help-echo stash-help-echo)
-	(mapconcat
-	 (lambda (x)
-	   (propertize x
-		       'face 'font-lock-variable-name-face
-		       'mouse-face 'highlight
-		       'help-echo "mouse-3: Show stash menu\nRET: Show stash\nA: Apply stash\nP: Apply and remove stash (pop)\nC-k: Delete stash"
-		       'keymap vc-git-stash-map))
-	 stash "\n"))
+        (propertize "\nStash      : " 'face 'font-lock-type-face
+                    'help-echo stash-help-echo)
+        stash-button
+        stash-string)
        (concat
-	(propertize "Stash      : " 'face 'font-lock-type-face
+	(propertize "\nStash      : " 'face 'font-lock-type-face
 		    'help-echo stash-help-echo)
 	(propertize "Nothing stashed"
 		    'help-echo stash-help-echo
-- 
2.23.0

Re: [PATCH] Allow hiding stashes from vc-dir

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> Toggling with a configurable limit implemented in attached. I did feel
> like I was reinventing the wheel a bit, is there no "create a bunch of
> strings/regions and apply properties to them" library?

I don't think so?

Looks good to me, but one minor comment:

> +(declare-function cl-subseq "cl-extra" (seq start &optional end))

This functions is ;;;###autoload in cl-extra.el, so I would have thought
that this wouldn't be necessary...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Fri, 18 Oct 2019 05:14:27 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    >> +(declare-function cl-subseq "cl-extra" (seq start &optional end))

    Lars> This functions is ;;;###autoload in cl-extra.el, so I would have thought
    Lars> that this wouldn't be necessary...

I thought so too, but:

  ELC      vc/vc-git.elc

In end of data:
vc/vc-git.el:1780:1:Warning: the function ‘cl-subseq’ might not be defined at
    runtime.

Re: [PATCH] Allow hiding stashes from vc-dir

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 18 Oct 2019 10:39:21 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org,
>  Juri Linkov <juri@linkov.net>
> 
>     >> +(declare-function cl-subseq "cl-extra" (seq start &optional end))
> 
>     Lars> This functions is ;;;###autoload in cl-extra.el, so I would have thought
>     Lars> that this wouldn't be necessary...
> 
> I thought so too, but:
> 
>   ELC      vc/vc-git.elc
> 
> In end of data:
> vc/vc-git.el:1780:1:Warning: the function ‘cl-subseq’ might not be defined at
>     runtime.

Because of this part in cl-extra.el:

  ;; Local variables:
  ;; generated-autoload-file: "cl-loaddefs.el"

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Fri, 18 Oct 2019 12:01:13 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> I thought so too, but:
    >> 
    >> ELC      vc/vc-git.elc
    >> 
    >> In end of data:
    >> vc/vc-git.el:1780:1:Warning: the function ‘cl-subseq’ might not be defined at
    >> runtime.

    Eli> Because of this part in cl-extra.el:

    Eli>   ;; Local variables:
    Eli>   ;; generated-autoload-file: "cl-loaddefs.el"


But cl-lib does:

(provide 'cl-lib)
(unless (load "cl-loaddefs" 'noerror 'quiet)
  ;; When bootstrapping, cl-loaddefs hasn't been built yet!
  (require 'cl-macs)
  (require 'cl-seq))

Is that not enough? Or (more likely) I donʼt understand the whole cl
load/autoload stuff.

Robert

Re: [PATCH] Allow hiding stashes from vc-dir

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  juri@linkov.net,  emacs-devel@gnu.org
> Date: Fri, 18 Oct 2019 11:25:12 +0200
> 
>     >> ELC      vc/vc-git.elc
>     >> 
>     >> In end of data:
>     >> vc/vc-git.el:1780:1:Warning: the function ‘cl-subseq’ might not be defined at
>     >> runtime.
> 
>     Eli> Because of this part in cl-extra.el:
> 
>     Eli>   ;; Local variables:
>     Eli>   ;; generated-autoload-file: "cl-loaddefs.el"
> 
> 
> But cl-lib does:
> 
> (provide 'cl-lib)
> (unless (load "cl-loaddefs" 'noerror 'quiet)
>   ;; When bootstrapping, cl-loaddefs hasn't been built yet!
>   (require 'cl-macs)
>   (require 'cl-seq))

Does this happen during byte-compilation?

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Fri, 18 Oct 2019 12:31:35 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> (provide 'cl-lib)
    >> (unless (load "cl-loaddefs" 'noerror 'quiet)
    >> ;; When bootstrapping, cl-loaddefs hasn't been built yet!
    >> (require 'cl-macs)
    >> (require 'cl-seq))

    Eli> Does this happen during byte-compilation?

I think so. With the following patch:

diff --git a/lisp/emacs-lisp/cl-lib.el b/lisp/emacs-lisp/cl-lib.el
index ff09691817..4238f09c1b 100644
--- a/lisp/emacs-lisp/cl-lib.el
+++ b/lisp/emacs-lisp/cl-lib.el
@@ -627,7 +627,7 @@ substring
 ;;; Miscellaneous.
 
 (provide 'cl-lib)
-(unless (load "cl-loaddefs" 'noerror 'quiet)
+(unless (load "cl-loaddefs" 'noerror)
   ;; When bootstrapping, cl-loaddefs hasn't been built yet!
   (require 'cl-macs)
   (require 'cl-seq))

I get:

make[2]: Nothing to be done for `all'.
  ELC      emacs-lisp/cl-lib.elc
Loading /Users/rpluim/repos/emacs-real-master/lisp/emacs-lisp/cl-loaddefs.el (source)...

Re: [PATCH] Allow hiding stashes from vc-dir

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  emacs-devel@gnu.org,  juri@linkov.net
> Date: Fri, 18 Oct 2019 12:18:19 +0200
> 
> >>>>> On Fri, 18 Oct 2019 12:31:35 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> (provide 'cl-lib)
>     >> (unless (load "cl-loaddefs" 'noerror 'quiet)
>     >> ;; When bootstrapping, cl-loaddefs hasn't been built yet!
>     >> (require 'cl-macs)
>     >> (require 'cl-seq))
> 
>     Eli> Does this happen during byte-compilation?
> 
> I think so. With the following patch:
> 
> diff --git a/lisp/emacs-lisp/cl-lib.el b/lisp/emacs-lisp/cl-lib.el
> index ff09691817..4238f09c1b 100644
> --- a/lisp/emacs-lisp/cl-lib.el
> +++ b/lisp/emacs-lisp/cl-lib.el
> @@ -627,7 +627,7 @@ substring
>  ;;; Miscellaneous.
>  
>  (provide 'cl-lib)
> -(unless (load "cl-loaddefs" 'noerror 'quiet)
> +(unless (load "cl-loaddefs" 'noerror)
>    ;; When bootstrapping, cl-loaddefs hasn't been built yet!
>    (require 'cl-macs)
>    (require 'cl-seq))
> 
> I get:
> 
> make[2]: Nothing to be done for `all'.
>   ELC      emacs-lisp/cl-lib.elc
> Loading /Users/rpluim/repos/emacs-real-master/lisp/emacs-lisp/cl-loaddefs.el (source)...

Then it sounds like more debugging is needed to understand why
autoloading seems to not work in this case.

Thanks.

Re: [PATCH] Allow hiding stashes from vc-dir

[Stefan Monnier]

> +(declare-function cl-subseq "cl-extra" (seq start &optional end))

Wrong fix: if you want to use cl-lib functions, then you need to
(require 'cl-lib) outside of an eval-when-compile.  IOW you need:

    diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
    index 9715aea1fd..aa3b41454d 100644
    --- a/lisp/vc/vc-git.el
    +++ b/lisp/vc/vc-git.el
    @@ -102,8 +102,8 @@
     
     ;;; Code:
     
    +(require 'cl-lib)
     (eval-when-compile
    -  (require 'cl-lib)
       (require 'subr-x) ; for string-trim-right
       (require 'vc)
       (require 'vc-dir))


-- Stefan

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Fri, 18 Oct 2019 15:22:46 +0300, Eli Zaretskii <eliz@gnu.org> said:
    Eli> Then it sounds like more debugging is needed to understand why
    Eli> autoloading seems to not work in this case.

I donʼt understand what's going on:

In vc-git.el I put:

    (eval-when-compile
      (message "cl-sub: '%s' '%s' '%s'" #'cl-subseq (symbol-file #'cl-subseq) (symbol-function #'cl-subseq)))

Which outputs:

    make[2]: Nothing to be done for `all'.
      ELC      vc/vc-git.elc
    Loading /Users/rpluim/repos/emacs-real-master/lisp/emacs-lisp/cl-loaddefs.el (source)...
    cl-sub: ’cl-subseq’ ’cl-extra’ ’(autoload cl-extra Return the subsequence of SEQ from START to END.
    If END is omitted, it defaults to the length of the sequence.
    If START or END is negative, it counts from the end.
    Signal an error if START or END are outside of the sequence (i.e
    too large if positive or too small if negative).

    (fn SEQ START &optional END) nil nil)’

    In end of data:
    vc/vc-git.el:1782:1:Warning: the function ‘cl-subseq’ might not be defined at
        runtime.

So the function has a correct autoload specification.

Robert

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Fri, 18 Oct 2019 09:22:22 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >> +(declare-function cl-subseq "cl-extra" (seq start &optional end))
    Stefan> Wrong fix: if you want to use cl-lib functions, then you need to
    Stefan> (require 'cl-lib) outside of an eval-when-compile.  IOW you need:

    Stefan>     diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
    Stefan>     index 9715aea1fd..aa3b41454d 100644
    Stefan>     --- a/lisp/vc/vc-git.el
    Stefan>     +++ b/lisp/vc/vc-git.el
    Stefan>     @@ -102,8 +102,8 @@
     
    Stefan>      ;;; Code:
     
    Stefan>     +(require 'cl-lib)
    Stefan>      (eval-when-compile
    Stefan>     -  (require 'cl-lib)
    Stefan>        (require 'subr-x) ; for string-trim-right
    Stefan>        (require 'vc)
    Stefan>        (require 'vc-dir))

I believe you [1]. Although in my mind the whole point of the autoload
cookies is that you donʼt need to require the whole of cl-lib just to
use one of its functions.

I guess with that itʼs ready to push from my end. Lars, does it work
the way you want?

Footnotes:
[1]  I did test that it works, though :-)

Re: [PATCH] Allow hiding stashes from vc-dir

[Stefan Monnier]

> I believe you [1]. Although in my mind the whole point of the autoload
> cookies is that you donʼt need to require the whole of cl-lib just to
> use one of its functions.

The autoload cookies in cl-extra.el are setup such that it means "once
cl-lib is loaded, those functions get autoloaded".  Which is why you
still need to load cl-lib in order to setup the autoload.

But really, you don't need to know any of that.  cl-subseq is a function
exported by cl-lib, so you need to load cl-lib to use the function (the
fact that it uses an autoload is a detail internal to cl-lib, just like
the fact that it's not defined in cl-lib.el but in cl-extra.el).


        Stefan

Re: [PATCH] Allow hiding stashes from vc-dir

[Stefan Monnier]

>     Eli> Then it sounds like more debugging is needed to understand why
>     Eli> autoloading seems to not work in this case.
>
> I donʼt understand what's going on:

Re-read the warning:

    vc/vc-git.el:1782:1:Warning: the function ‘cl-subseq’ might not be
        defined at runtime.

It doesn't say cl-subseq is an unknown function, but that it suspects
that the cl-subseq function while currently defined (during the
compilation) may not be defined when the code will be used.

That's because the code that loaded it was wrapped in
a `eval-when-compile` so it was labeled as "defined now but maybe not at
runtime".


        Stefan

Re: [PATCH] Allow hiding stashes from vc-dir

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> I guess with that itʼs ready to push from my end. Lars, does it work
> the way you want?

Yes, works very nice.  It'd be good if it remembered the decision to
show/hide the stashes after hitting `g', but that can be tweaked later.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [PATCH] Allow hiding stashes from vc-dir

[Andreas Schwab]

On Okt 18 2019, Robert Pluim <rpluim@gmail.com> wrote:

> So the function has a correct autoload specification.

Only when cl-lib is loaded.  If it isn't, then cl-subseq is undefined.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Sat, 19 Oct 2019 10:15:28 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Robert Pluim <rpluim@gmail.com> writes:
    >> I guess with that itʼs ready to push from my end. Lars, does it work
    >> the way you want?

    Lars> Yes, works very nice.  It'd be good if it remembered the decision to
    Lars> show/hide the stashes after hitting `g', but that can be tweaked later.

What kind of maniac hits 'g' in a vc-dir buffer? ;-)

Pushed as a5ca89d35c

Robert

Re: [PATCH] Allow hiding stashes from vc-dir

[Robert Pluim]

>>>>> On Fri, 18 Oct 2019 23:19:47 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    Eli> Then it sounds like more debugging is needed to understand why
    Eli> autoloading seems to not work in this case.
    >> 
    >> I donʼt understand what's going on:

    Stefan> Re-read the warning:

    Stefan>     vc/vc-git.el:1782:1:Warning: the function ‘cl-subseq’ might not be
    Stefan>         defined at runtime.

    Stefan> It doesn't say cl-subseq is an unknown function, but that it suspects
    Stefan> that the cl-subseq function while currently defined (during the
    Stefan> compilation) may not be defined when the code will be used.

    Stefan> That's because the code that loaded it was wrapped in
    Stefan> a `eval-when-compile` so it was labeled as "defined now but maybe not at
    Stefan> runtime".

Thanks Stefan (and Andreas). I now understand what's going on.

Robert