bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Jim Porter]

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

This patch adds support for outline-minor-mode to Eshell. When enabled, 
this will just add outline buttons to the prompt (as the top level), and 
one to the start of the output (as level 2).

The Eshell side is hopefully fairly straightforward, but the outline.el 
part probably warrants a close review. The changes in outline.el are all 
there to support outline headings that are just a newline and nothing 
else. This is important for both the output (you might call a command 
that prints a newline as the first character), and the prompt (the first 
character of your prompt could be a newline as a way of separating the 
prompt from the output just above it).[1]

Mainly, the changes in outline.el are about checking to see if the 
character *before* point is invisible (i.e. part of a collapsed node). 
This way, if point is at the beginning of the output, and you collapse 
that node, outline.el still considers you to be on a heading. (Without 
these changes, cycling a node doesn't work correctly.)

I'm open to adding regression tests here since the logic is pretty 
subtle, but I didn't see any existing ones for outline.el and it's not 
immediately obvious the best way to test its behavior. Let me know if 
anyone has ideas though.

[1] In the prompt case, a different way to solve this would be to treat 
the start of the heading as the first non-empty line. That doesn't work 
for output though since *all* lines could be empty.

[-- Attachment #2: 0001-Add-support-for-outline-minor-mode-in-Eshell.patch --]
[-- Type: text/plain, Size: 7299 bytes --]

From f10b887f945d057a50a06efccfc9d8c1e6762f0e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 30 May 2024 14:31:55 -0700
Subject: [PATCH] Add support for outline-minor-mode in Eshell

* lisp/outline.el (outline-back-to-heading): Skip past invisible text.
(outline-on-heading-p): Check for invisibility of the character *before*
point; that way, this returns non-nil if at the end the first line of a
collapsed section.
(outline-flag-region): Set the outline button as needed.
(outline-end-of-subtree): Allow for stopping at empty lines.

* lisp/eshell/em-prompt.el (eshell-outline-search)
(eshell-outline-level): New functions...
(eshell-prompt-initialize): ... use them.

* doc/misc/eshell.texi (Bugs and ideas): Remove implemented idea.

* etc/NEWS: Announce this change.
---
 doc/misc/eshell.texi     |  6 ------
 etc/NEWS                 |  3 +++
 lisp/eshell/em-prompt.el | 45 +++++++++++++++++++++++++++++++++++++++-
 lisp/outline.el          | 29 ++++++++++++++++++--------
 4 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 873d14aff32..071a2e59191 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2967,12 +2967,6 @@ Bugs and ideas
 
 @item Support zsh's ``Parameter Expansion'' syntax, i.e., @samp{$@{@var{name}:-@var{val}@}}
 
-@item Create a mode @code{eshell-browse}
-
-It would treat the Eshell buffer as an outline.  Collapsing the outline
-hides all of the output text.  Collapsing again would show only the
-first command run in each directory
-
 @item Allow other revisions of a file to be referenced using @samp{file@{rev@}}
 
 This would be expanded by @code{eshell-expand-file-name} (see above).
diff --git a/etc/NEWS b/etc/NEWS
index 3c672ffed8f..b5d4c9cfe6e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -877,6 +877,9 @@ can make it executable like other shell scripts:
 
     #!/usr/bin/env -S emacs --batch -f eshell-batch-file
 
+---
+*** Eshell now supports 'outline-minor-mode'.
+
 +++
 *** New builtin Eshell command 'compile'.
 This command runs another command, sending its output to a compilation
diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index b6556d29544..2e637acc05f 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -107,6 +107,45 @@ eshell-prompt-repeat-map
 
 ;;; Functions:
 
+(defun eshell-outline-search (&optional bound move backward looking-at)
+  "Search for outline headings.  See `outline-search-function'."
+  (let ((value '(prompt command-output)))
+    (if looking-at
+        (when (and (memq (get-text-property (point) 'field) value)
+                   (= (field-beginning (point)) (point)))
+          (set-match-data (list (pos-bol) (pos-eol)))
+          t)
+      ;; Go to the end when in the middle of heading.
+      (when (and (not backward)
+                 (memq (get-text-property (point) 'field) value)
+                 (/= (point) (field-beginning (point))))
+        (goto-char (field-end (point))))
+      ;; Search for our wanted fields.
+      (if-let ((prop-match
+                (funcall (if backward #'text-property-search-backward
+                           #'text-property-search-forward)
+                         'field value (lambda (list elt) (memq elt list))))
+               ;; beg and end should only include the first line of the field.
+               (beg (prop-match-beginning prop-match))
+               (end (save-excursion (goto-char beg) (pos-eol)))
+               ((or (null bound) (if backward (>= beg bound) (<= end bound))))
+               ((save-excursion
+                  (goto-char beg)
+                  (= (field-beginning (point) (pos-bol))))))
+          (progn
+            (goto-char (if backward beg end))
+            (set-match-data (list beg end))
+            t)
+        (when move (goto-char (or bound (if backward (point-min) (point-max)))))
+        nil))))
+
+(defun eshell-outline-level ()
+  "Get the outline level at point."
+  (pcase (get-text-property (point) 'field)
+    ('prompt 1)
+    ('command-output 2)
+    (_ (error "Not at an outline heading"))))
+
 (define-minor-mode eshell-prompt-mode
   "Minor mode for eshell-prompt module.
 
@@ -117,7 +156,11 @@ eshell-prompt-initialize
   "Initialize the prompting code."
   (unless eshell-non-interactive-p
     (add-hook 'eshell-post-command-hook 'eshell-emit-prompt nil t)
-    (eshell-prompt-mode)))
+    (eshell-prompt-mode)
+
+    (setq-local outline-search-function #'eshell-outline-search
+                outline-level #'eshell-outline-level
+                outline-minor-mode-use-buttons 'in-margins)))
 
 (defun eshell-emit-prompt ()
   "Emit a prompt if eshell is being used interactively."
diff --git a/lisp/outline.el b/lisp/outline.el
index 40a75701cbf..b79984092e9 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -687,6 +687,11 @@ outline-back-to-heading
   "Move to previous heading line, or beg of this line if it's a heading.
 Only visible heading lines are considered, unless INVISIBLE-OK is non-nil."
   (forward-line 0)
+  (when (and (not invisible-ok)
+             (> (point) (point-min))
+             (get-char-property (1- (point)) 'invisible))
+    ;; Skip backwards past any invisible text.
+    (goto-char (previous-single-char-property-change (point) 'invisible)))
   (or (outline-on-heading-p invisible-ok)
       (let (found)
 	(save-excursion
@@ -706,7 +711,9 @@ outline-on-heading-p
 If INVISIBLE-OK is non-nil, an invisible heading line is ok too."
   (save-excursion
     (forward-line 0)
-    (and (bolp) (or invisible-ok (not (outline-invisible-p)))
+    (and (bolp) (or invisible-ok
+                    (bobp)
+                    (not (outline-invisible-p (1- (point)))))
 	 (if outline-search-function
              (funcall outline-search-function nil nil nil t)
            (looking-at outline-regexp)))))
@@ -1004,6 +1011,11 @@ outline-flag-region
     (let ((o (make-overlay from to nil 'front-advance)))
       (overlay-put o 'evaporate t)
       (overlay-put o 'invisible 'outline)
+      (unless (eq outline-minor-mode-use-buttons 'insert)
+        ;; Set up the button on this overlay too.  This ensures that the
+        ;; button is visible even if the first line of the region is
+        ;; empty.
+        (overlay-put o 'before-string (nth 1 outline--button-icons)))
       (overlay-put o 'isearch-open-invisible
 		   (or outline-isearch-open-invisible-function
 		       #'outline-isearch-open-invisible))))
@@ -1264,14 +1276,13 @@ outline-end-of-subtree
 		(or first (> (funcall outline-level) level)))
       (setq first nil)
       (outline-next-heading))
-    (if (and (bolp) (not (eolp)))
-	;; We stopped at a nonempty line (the next heading).
-	(progn
-	  ;; Go to end of line before heading
-	  (forward-char -1)
-          (if (and outline-blank-line (bolp))
- 	      ;; leave blank line before heading
- 	      (forward-char -1))))))
+    (when (bolp)
+      ;; We stopped at the next heading.  Go to end of line before
+      ;; heading.
+      (forward-char -1)
+      (when (and outline-blank-line (bolp))
+        ;; Leave blank line before heading.
+        (forward-char -1)))))
 \f
 (defun outline-show-branches ()
   "Show all subheadings of this heading, but not their bodies."
-- 
2.25.1

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Eli Zaretskii]

> Cc: juri@linkov.net
> Date: Thu, 30 May 2024 22:18:44 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> 
> This patch adds support for outline-minor-mode to Eshell. When enabled, 
> this will just add outline buttons to the prompt (as the top level), and 
> one to the start of the output (as level 2).

Thanks.

> diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
> index 873d14aff32..071a2e59191 100644
> --- a/doc/misc/eshell.texi
> +++ b/doc/misc/eshell.texi
> @@ -2967,12 +2967,6 @@ Bugs and ideas
>  
>  @item Support zsh's ``Parameter Expansion'' syntax, i.e., @samp{$@{@var{name}:-@var{val}@}}
>  
> -@item Create a mode @code{eshell-browse}
> -
> -It would treat the Eshell buffer as an outline.  Collapsing the outline
> -hides all of the output text.  Collapsing again would show only the
> -first command run in each directory
> -
>  @item Allow other revisions of a file to be referenced using @samp{file@{rev@}}
>  
>  This would be expanded by @code{eshell-expand-file-name} (see above).
> diff --git a/etc/NEWS b/etc/NEWS
> index 3c672ffed8f..b5d4c9cfe6e 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -877,6 +877,9 @@ can make it executable like other shell scripts:
>  
>      #!/usr/bin/env -S emacs --batch -f eshell-batch-file
>  
> +---
> +*** Eshell now supports 'outline-minor-mode'.

This is what I call "anti-documentation": the text you removed, which
described a non-existing feature, told more about the feature than the
text you added after implementing it ;-)

Seriously, though: since it is completely un-obvious what would
outline-minor-mode mean for command-line-oriented modes like Eshell,
both NEWS and the Eshell manual should say at least a couple of words
about what that does.  We cannot get away by just mentioning the
support and keeping silent about the rest.

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Juri Linkov]

> This patch adds support for outline-minor-mode to Eshell. When enabled,
> this will just add outline buttons to the prompt (as the top level), and
> one to the start of the output (as level 2).

I'm not familiar with Eshell, so I tried in shell-mode to set
(setq-local outline-regexp "^.*\\$ ") and it works nicely
(for the real use it should be based on 'shell-prompt-pattern').

But I believe you had a good reason to prefer outline-search-function
and more changes in outline.el to support outlines in Eshell.

> The Eshell side is hopefully fairly straightforward, but the outline.el
> part probably warrants a close review. The changes in outline.el are all
> there to support outline headings that are just a newline and nothing
> else. This is important for both the output (you might call a command that
> prints a newline as the first character), and the prompt (the first
> character of your prompt could be a newline as a way of separating the
> prompt from the output just above it).[1]

I don't understand why do you need an outline heading for the empty line.
Is it really useful for hide/show an empty line?

> Mainly, the changes in outline.el are about checking to see if the
> character *before* point is invisible (i.e. part of a collapsed node). This
> way, if point is at the beginning of the output, and you collapse that
> node, outline.el still considers you to be on a heading. (Without these
> changes, cycling a node doesn't work correctly.)
>
> I'm open to adding regression tests here since the logic is pretty subtle,
> but I didn't see any existing ones for outline.el and it's not immediately
> obvious the best way to test its behavior. Let me know if anyone has ideas
> though.

Adding regression tests would be nice since the changes in outline.el
don't look trivial.  I guess tests for outline.el should do two basic things:
call various outline commands (perhaps just outline-cycle and outline-cycle-buffer
should be sufficient) with various settings of outline variables,
then 'invisible-p' could check if outline visibility is satisfied.
Or we could prefer a lazy approach to install changes, then to add tests
only in case when a regression will be discovered later.

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Jim Porter]

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

On 5/30/2024 11:05 PM, Eli Zaretskii wrote:
> Seriously, though: since it is completely un-obvious what would
> outline-minor-mode mean for command-line-oriented modes like Eshell,
> both NEWS and the Eshell manual should say at least a couple of words
> about what that does.  We cannot get away by just mentioning the
> support and keeping silent about the rest.

Good point. I'd tried to write some more docs previously but was very 
unsatisfied with my attempt. After sleeping on it, I've tried again (see 
attached).

I added a preliminary commit here to move some of the existing docs into 
a new "Interaction" section, since I think the manual should really 
split out the documentation of commands and their syntax from how to use 
Eshell *interactively*. (I split this out since the diff is fairly 
large, but the changes aren't all that exciting: it just moves things 
from one spot to another, and adds some new index entries.)

That provides a better spot to explain this new outline-minor-mode 
support. I also filled in documentation for a few of the existing 
navigation commands available in Eshell.

[-- Attachment #2: 0001-Add-an-Interaction-chapter-to-the-Eshell-manual.patch --]
[-- Type: text/plain, Size: 10586 bytes --]

From 8874fd84fc340eb96357c36aac9d04336643d073 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 31 May 2024 09:36:03 -0700
Subject: [PATCH 1/2] Add an "Interaction" chapter to the Eshell manual

* doc/misc/eshell.texi (Interaction): New chapter.
(Completion, History): Move into "Interaction" and add key indexing.
(Key rebinding): Add key indexing.
(Command Index): Add this index.
---
 doc/misc/eshell.texi | 204 +++++++++++++++++++++++--------------------
 1 file changed, 110 insertions(+), 94 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 873d14aff32..2400ba988b4 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -79,15 +79,14 @@ Top
 * Commands::
 * Expansion::
 * Input/Output::
+* Interaction::
 * Extension modules::
 * Bugs and ideas::              Known problems, and future ideas.
 * GNU Free Documentation License:: The license for this documentation.
 * Concept Index::
 * Function and Variable Index::
 * Command Index::
-@ignore
 * Key Index::
-@end ignore
 @end menu
 
 @node Introduction
@@ -208,8 +207,6 @@ Commands
 * Variables::
 * Aliases::
 * Remote Access::
-* History::
-* Completion::
 * Control Flow::
 * Scripts::
 @end menu
@@ -1556,92 +1553,6 @@ Remote Access
 you can set the option @code{eshell-explicit-remote-commands} to
 @code{nil}.
 
-@node History
-@section History
-@cmindex history
-@vindex eshell-history-size
-The @samp{history} command shows all commands kept in the history ring
-as numbered list.  If the history ring contains
-@code{eshell-history-size} commands, those numbers change after every
-command invocation, therefore the @samp{history} command shall be
-applied before using the expansion mechanism with history numbers.
-
-The n-th entry of the history ring can be applied with the @samp{!n}
-command.  If @code{n} is negative, the entry is counted from the end
-of the history ring.
-
-@cindex event designators
-@findex eshell-expand-history-references
-When history event designators are enabled (by adding
-@code{eshell-expand-history-references} to
-@code{eshell-expand-input-functions}), @samp{!foo} expands to the last
-command beginning with @code{foo}, and @samp{!?foo} to the last
-command containing @code{foo}.  The n-th argument of the last command
-beginning with @code{foo} is accessible by @code{!foo:n}.
-
-@vindex eshell-history-file-name
-@vindex eshell-history-append
-The history is loaded to the history ring from the file
-@code{eshell-history-file-name} at the start of every session, and
-saved to that file at the end of every session.  The default history
-saving behavior is to overwrite the history file with the whole
-history ring of the session.  If @code{eshell-history-append} is
-non-@code{nil}, the history will instead be saved by appending new
-entries from the session to the history file, which could prevent
-potential history loss with multiple Eshell sessions.  Unlike other
-shells, such as Bash, Eshell cannot currently be configured to control
-the size of the history file.  In particular, when
-@code{eshell-history-append} is non-@code{nil}, the size of the file
-will keep increasing, and the recommended way to truncate the file is
-to run the @samp{history -w} command in an Eshell session.
-
-Since the default buffer navigation and searching key-bindings are
-still present in the Eshell buffer, the commands for history
-navigation and searching are bound to different keys:
-
-@table @kbd
-@item M-r
-@itemx M-s
-History I-search.
-
-@item M-p
-@itemx M-n
-Previous and next history line.  If there is anything on the input
-line when you run these commands, they will instead jump to the
-previous or next line that begins with that string.
-@end table
-
-@node Completion
-@section Completion
-Eshell uses the pcomplete package for programmable completion, similar
-to that of other command shells.  Argument completion differs depending
-on the preceding command: for example, possible completions for
-@command{rmdir} are only directories, while @command{rm} completions can
-be directories @emph{and} files.  Eshell provides predefined completions
-for the built-in functions and some common external commands, and you
-can define your own for any command.
-
-Eshell completion also works for Lisp forms and glob patterns. If the
-point is on a Lisp form, then @key{TAB} will behave similarly to
-completion in @code{elisp-mode} and @code{lisp-interaction-mode}.  For
-glob patterns, the pattern will be removed from the input line, and
-replaced by the completion.
-
-If you want to see the entire list of possible completions (e.g. when it's
-below the @code{completion-cycle-threshold}), press @kbd{M-?}.
-
-@subsection pcomplete
-Pcomplete, short for programmable completion, is the completion
-library originally written for Eshell, but usable for command
-completion@footnote{Command completion, as opposed to code completion,
-which is beyond the scope of pcomplete.} in other modes.
-
-Completions are defined as functions (with @code{defun}) named
-@code{pcomplete/COMMAND}, where @code{COMMAND} is the name of the
-command for which this function provides completions; you can also name
-the function @code{pcomplete/MAJOR-MODE/COMMAND} to define completions
-for a specific major mode.
-
 @node Control Flow
 @section Control Flow
 Because Eshell commands can not (easily) be combined with Lisp forms,
@@ -2486,6 +2397,111 @@ Pipelines
  sh -c "foo | baz" >#<buffer quux>
 @end example
 
+@node Interaction
+@chapter Interaction
+
+As an interactive shell, Eshell contains many features for helping to
+interact with your command environment.  In addition, since Eshell is a
+part of Emacs, all of the usual Emacs commands work within Eshell as
+well.
+
+@menu
+* Completion::
+* History::
+@end menu
+
+@node Completion
+@section Completion
+Eshell uses the pcomplete package for programmable completion, similar
+to that of other command shells.  Argument completion differs depending
+on the preceding command: for example, possible completions for
+@command{rmdir} are only directories, while @command{rm} completions can
+be directories @emph{and} files.  Eshell provides predefined completions
+for the built-in functions and some common external commands, and you
+can define your own for any command.
+
+@kindex TAB
+Eshell completion also works for Lisp forms and glob patterns. If the
+point is on a Lisp form, then @key{TAB} will behave similarly to
+completion in @code{elisp-mode} and @code{lisp-interaction-mode}.  For
+glob patterns, the pattern will be removed from the input line, and
+replaced by the completion.
+
+@kindex M-?
+If you want to see the entire list of possible completions (e.g. when it's
+below the @code{completion-cycle-threshold}), press @kbd{M-?}.
+
+@subsection pcomplete
+Pcomplete, short for programmable completion, is the completion
+library originally written for Eshell, but usable for command
+completion@footnote{Command completion, as opposed to code completion,
+which is beyond the scope of pcomplete.} in other modes.
+
+Completions are defined as functions (with @code{defun}) named
+@code{pcomplete/COMMAND}, where @code{COMMAND} is the name of the
+command for which this function provides completions; you can also name
+the function @code{pcomplete/MAJOR-MODE/COMMAND} to define completions
+for a specific major mode.
+
+@node History
+@section History
+@cmindex history
+@vindex eshell-history-size
+The @samp{history} command shows all commands kept in the history ring
+as numbered list.  If the history ring contains
+@code{eshell-history-size} commands, those numbers change after every
+command invocation, therefore the @samp{history} command shall be
+applied before using the expansion mechanism with history numbers.
+
+The n-th entry of the history ring can be applied with the @samp{!n}
+command.  If @code{n} is negative, the entry is counted from the end
+of the history ring.
+
+@cindex event designators
+@findex eshell-expand-history-references
+When history event designators are enabled (by adding
+@code{eshell-expand-history-references} to
+@code{eshell-expand-input-functions}), @samp{!foo} expands to the last
+command beginning with @code{foo}, and @samp{!?foo} to the last
+command containing @code{foo}.  The n-th argument of the last command
+beginning with @code{foo} is accessible by @code{!foo:n}.
+
+@vindex eshell-history-file-name
+@vindex eshell-history-append
+The history is loaded to the history ring from the file
+@code{eshell-history-file-name} at the start of every session, and
+saved to that file at the end of every session.  The default history
+saving behavior is to overwrite the history file with the whole
+history ring of the session.  If @code{eshell-history-append} is
+non-@code{nil}, the history will instead be saved by appending new
+entries from the session to the history file, which could prevent
+potential history loss with multiple Eshell sessions.  Unlike other
+shells, such as Bash, Eshell cannot currently be configured to control
+the size of the history file.  In particular, when
+@code{eshell-history-append} is non-@code{nil}, the size of the file
+will keep increasing, and the recommended way to truncate the file is
+to run the @samp{history -w} command in an Eshell session.
+
+Since the default buffer navigation and searching key-bindings are
+still present in the Eshell buffer, the commands for history
+navigation and searching are bound to different keys:
+
+@table @kbd
+@kindex M-r
+@kindex M-s
+@item M-r
+@itemx M-s
+History I-search.
+
+@kindex M-p
+@kindex M-n
+@item M-p
+@itemx M-n
+Previous and next history line.  If there is anything on the input
+line when you run these commands, they will instead jump to the
+previous or next line that begins with that string.
+@end table
+
 @node Extension modules
 @chapter Extension modules
 Eshell provides a facility for defining extension modules so that they
@@ -2524,6 +2540,10 @@ Key rebinding
 input text.  To enable this module, add @code{eshell-rebind} to
 @code{eshell-modules-list}.
 
+@kindex C-u
+@kindex C-w
+@kindex C-p
+@kindex C-n
 For example, it binds @kbd{C-u} to kill the current input text and
 @kbd{C-w} to @code{backward-kill-word}.  If the history module is
 enabled, it also binds @kbd{C-p} and @kbd{C-n} to move through the
@@ -3174,13 +3194,9 @@ Command Index
 
 @printindex cm
 
-@c There are no @kindex entries in this manual; avoid generating an
-@c empty menu.
-@ignore
 @node Key Index
 @unnumbered Key Index
 
 @printindex ky
-@end ignore
 
 @bye
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-outline-minor-mode-in-Eshell.patch --]
[-- Type: text/plain, Size: 8642 bytes --]

From cf97f99314a74c9b40cb31252b970eb88705e4ac Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 30 May 2024 14:31:55 -0700
Subject: [PATCH 2/2] Add support for outline-minor-mode in Eshell

* lisp/outline.el (outline-back-to-heading): Skip past invisible text.
(outline-on-heading-p): Check for invisibility of the character *before*
point; that way, this returns non-nil if at the end the first line of a
collapsed section.
(outline-flag-region): Set the outline button as needed.
(outline-end-of-subtree): Allow for stopping at empty lines.

* lisp/eshell/em-prompt.el (eshell-outline-search)
(eshell-outline-level): New functions...
(eshell-prompt-initialize): ... use them.

* doc/misc/eshell.texi (Navigation): New section.
(Bugs and ideas): Remove implemented idea.

* etc/NEWS: Announce this change.
---
 doc/misc/eshell.texi     | 36 ++++++++++++++++++++++++++------
 etc/NEWS                 |  6 ++++++
 lisp/eshell/em-prompt.el | 45 +++++++++++++++++++++++++++++++++++++++-
 lisp/outline.el          | 29 ++++++++++++++++++--------
 4 files changed, 100 insertions(+), 16 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 2400ba988b4..ec1cc5b2394 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -2406,10 +2406,40 @@ Interaction
 well.
 
 @menu
+* Navigation::
 * Completion::
 * History::
 @end menu
 
+@node Navigation
+@section Navigation
+
+@table @kbd
+
+@kindex C-c C-n
+@kindex C-c C-p
+@item C-c C-n
+@itemx C-c C-p
+Move point to the beginning of the input for the next or previous
+prompt.  With a prefix argument, move to the n-th next or previous
+prompt.
+
+@kindex C-c C-r
+@kindex C-M-l
+@item C-c C-r
+@itemx C-M-l
+Move point to the start of the previous command's output and display it
+at the top of the window.  With a prefix argument, this also narrows the
+region to the last command's output.
+
+@end table
+
+Eshell also supports Outline minor mode (@pxref{Outline Minor Mode, , ,
+emacs, The Emacs Editor}).  By enabling @code{outline-minor-mode},
+Eshell will allow collapsing commands and/or their output.  Each command
+is a top-level outline node, and the output of the command is a
+second-level node under the command.
+
 @node Completion
 @section Completion
 Eshell uses the pcomplete package for programmable completion, similar
@@ -2987,12 +3017,6 @@ Bugs and ideas
 
 @item Support zsh's ``Parameter Expansion'' syntax, i.e., @samp{$@{@var{name}:-@var{val}@}}
 
-@item Create a mode @code{eshell-browse}
-
-It would treat the Eshell buffer as an outline.  Collapsing the outline
-hides all of the output text.  Collapsing again would show only the
-first command run in each directory
-
 @item Allow other revisions of a file to be referenced using @samp{file@{rev@}}
 
 This would be expanded by @code{eshell-expand-file-name} (see above).
diff --git a/etc/NEWS b/etc/NEWS
index 3c672ffed8f..9c12978128d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -877,6 +877,12 @@ can make it executable like other shell scripts:
 
     #!/usr/bin/env -S emacs --batch -f eshell-batch-file
 
++++
+*** Eshell now supports 'outline-minor-mode'.
+By enabling 'outline-minor-mode', Eshell will allow collapsing commands
+and/or their output.  Each command is a top-level outline node, and the
+output of the command is a second-level node under the command.
+
 +++
 *** New builtin Eshell command 'compile'.
 This command runs another command, sending its output to a compilation
diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index b6556d29544..2e637acc05f 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -107,6 +107,45 @@ eshell-prompt-repeat-map
 
 ;;; Functions:
 
+(defun eshell-outline-search (&optional bound move backward looking-at)
+  "Search for outline headings.  See `outline-search-function'."
+  (let ((value '(prompt command-output)))
+    (if looking-at
+        (when (and (memq (get-text-property (point) 'field) value)
+                   (= (field-beginning (point)) (point)))
+          (set-match-data (list (pos-bol) (pos-eol)))
+          t)
+      ;; Go to the end when in the middle of heading.
+      (when (and (not backward)
+                 (memq (get-text-property (point) 'field) value)
+                 (/= (point) (field-beginning (point))))
+        (goto-char (field-end (point))))
+      ;; Search for our wanted fields.
+      (if-let ((prop-match
+                (funcall (if backward #'text-property-search-backward
+                           #'text-property-search-forward)
+                         'field value (lambda (list elt) (memq elt list))))
+               ;; beg and end should only include the first line of the field.
+               (beg (prop-match-beginning prop-match))
+               (end (save-excursion (goto-char beg) (pos-eol)))
+               ((or (null bound) (if backward (>= beg bound) (<= end bound))))
+               ((save-excursion
+                  (goto-char beg)
+                  (= (field-beginning (point) (pos-bol))))))
+          (progn
+            (goto-char (if backward beg end))
+            (set-match-data (list beg end))
+            t)
+        (when move (goto-char (or bound (if backward (point-min) (point-max)))))
+        nil))))
+
+(defun eshell-outline-level ()
+  "Get the outline level at point."
+  (pcase (get-text-property (point) 'field)
+    ('prompt 1)
+    ('command-output 2)
+    (_ (error "Not at an outline heading"))))
+
 (define-minor-mode eshell-prompt-mode
   "Minor mode for eshell-prompt module.
 
@@ -117,7 +156,11 @@ eshell-prompt-initialize
   "Initialize the prompting code."
   (unless eshell-non-interactive-p
     (add-hook 'eshell-post-command-hook 'eshell-emit-prompt nil t)
-    (eshell-prompt-mode)))
+    (eshell-prompt-mode)
+
+    (setq-local outline-search-function #'eshell-outline-search
+                outline-level #'eshell-outline-level
+                outline-minor-mode-use-buttons 'in-margins)))
 
 (defun eshell-emit-prompt ()
   "Emit a prompt if eshell is being used interactively."
diff --git a/lisp/outline.el b/lisp/outline.el
index 40a75701cbf..b79984092e9 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -687,6 +687,11 @@ outline-back-to-heading
   "Move to previous heading line, or beg of this line if it's a heading.
 Only visible heading lines are considered, unless INVISIBLE-OK is non-nil."
   (forward-line 0)
+  (when (and (not invisible-ok)
+             (> (point) (point-min))
+             (get-char-property (1- (point)) 'invisible))
+    ;; Skip backwards past any invisible text.
+    (goto-char (previous-single-char-property-change (point) 'invisible)))
   (or (outline-on-heading-p invisible-ok)
       (let (found)
 	(save-excursion
@@ -706,7 +711,9 @@ outline-on-heading-p
 If INVISIBLE-OK is non-nil, an invisible heading line is ok too."
   (save-excursion
     (forward-line 0)
-    (and (bolp) (or invisible-ok (not (outline-invisible-p)))
+    (and (bolp) (or invisible-ok
+                    (bobp)
+                    (not (outline-invisible-p (1- (point)))))
 	 (if outline-search-function
              (funcall outline-search-function nil nil nil t)
            (looking-at outline-regexp)))))
@@ -1004,6 +1011,11 @@ outline-flag-region
     (let ((o (make-overlay from to nil 'front-advance)))
       (overlay-put o 'evaporate t)
       (overlay-put o 'invisible 'outline)
+      (unless (eq outline-minor-mode-use-buttons 'insert)
+        ;; Set up the button on this overlay too.  This ensures that the
+        ;; button is visible even if the first line of the region is
+        ;; empty.
+        (overlay-put o 'before-string (nth 1 outline--button-icons)))
       (overlay-put o 'isearch-open-invisible
 		   (or outline-isearch-open-invisible-function
 		       #'outline-isearch-open-invisible))))
@@ -1264,14 +1276,13 @@ outline-end-of-subtree
 		(or first (> (funcall outline-level) level)))
       (setq first nil)
       (outline-next-heading))
-    (if (and (bolp) (not (eolp)))
-	;; We stopped at a nonempty line (the next heading).
-	(progn
-	  ;; Go to end of line before heading
-	  (forward-char -1)
-          (if (and outline-blank-line (bolp))
- 	      ;; leave blank line before heading
- 	      (forward-char -1))))))
+    (when (bolp)
+      ;; We stopped at the next heading.  Go to end of line before
+      ;; heading.
+      (forward-char -1)
+      (when (and outline-blank-line (bolp))
+        ;; Leave blank line before heading.
+        (forward-char -1)))))
 \f
 (defun outline-show-branches ()
   "Show all subheadings of this heading, but not their bodies."
-- 
2.25.1

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Jim Porter]

On 5/30/2024 11:51 PM, Juri Linkov wrote:
>> This patch adds support for outline-minor-mode to Eshell. When enabled,
>> this will just add outline buttons to the prompt (as the top level), and
>> one to the start of the output (as level 2).
> 
> I'm not familiar with Eshell, so I tried in shell-mode to set
> (setq-local outline-regexp "^.*\\$ ") and it works nicely
> (for the real use it should be based on 'shell-prompt-pattern').
> 
> But I believe you had a good reason to prefer outline-search-function
> and more changes in outline.el to support outlines in Eshell.

Yeah, I've been trying to reduce Eshell's reliance on simple regexps 
like that to determine the structure of the content. Since Eshell runs 
entirely within Emacs, we have the knowledge to authoritatively 
determine what parts of the buffer are what (you can do this in Eshell 
by examining the 'field' property). Regexps always run a (small?) risk 
of false-positives, and would additionally require users to update the 
regexp when they update their prompt.

>> The Eshell side is hopefully fairly straightforward, but the outline.el
>> part probably warrants a close review. The changes in outline.el are all
>> there to support outline headings that are just a newline and nothing
>> else. This is important for both the output (you might call a command that
>> prints a newline as the first character), and the prompt (the first
>> character of your prompt could be a newline as a way of separating the
>> prompt from the output just above it).[1]
> 
> I don't understand why do you need an outline heading for the empty line.
> Is it really useful for hide/show an empty line?

The main case I'm trying to account for is collapsing the command 
output. I think the most consistent way for Eshell is to make the 
"heading" always be the first line of the output, even if that line is 
empty. That way, the arrow button is always in the same spot 
(immediately after the prompt/input), and if the output had many leading 
empty lines, you can collapse all of them to save space (to be fair, 
this case probably isn't super-common).

For prompts, this isn't as important, since a single-line prompt should 
always have some visible text. For multi-line prompts, it would be 
possible to treat the heading as the first non-empty line, but that 
would be additional work on the Eshell side, and I think we'd still need 
the outline.el changes to handle collapsing the command output. 
(Improving heading-detection for multi-line prompts could always be done 
in a later bug, too.)

>> I'm open to adding regression tests here since the logic is pretty subtle,
>> but I didn't see any existing ones for outline.el and it's not immediately
>> obvious the best way to test its behavior. Let me know if anyone has ideas
>> though.
> 
> Adding regression tests would be nice since the changes in outline.el
> don't look trivial.  I guess tests for outline.el should do two basic things:
> call various outline commands (perhaps just outline-cycle and outline-cycle-buffer
> should be sufficient) with various settings of outline variables,
> then 'invisible-p' could check if outline visibility is satisfied.
> Or we could prefer a lazy approach to install changes, then to add tests
> only in case when a regression will be discovered later.

Thanks for the ideas. I think it would be good to add some tests here, 
so if you agree with my reasoning for the outline.el changes, I'll start 
writing a few regression tests.

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Eli Zaretskii]

> Date: Fri, 31 May 2024 12:33:32 -0700
> Cc: 71284@debbugs.gnu.org, juri@linkov.net
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 5/30/2024 11:05 PM, Eli Zaretskii wrote:
> > Seriously, though: since it is completely un-obvious what would
> > outline-minor-mode mean for command-line-oriented modes like Eshell,
> > both NEWS and the Eshell manual should say at least a couple of words
> > about what that does.  We cannot get away by just mentioning the
> > support and keeping silent about the rest.
> 
> Good point. I'd tried to write some more docs previously but was very 
> unsatisfied with my attempt. After sleeping on it, I've tried again (see 
> attached).
> 
> I added a preliminary commit here to move some of the existing docs into 
> a new "Interaction" section, since I think the manual should really 
> split out the documentation of commands and their syntax from how to use 
> Eshell *interactively*. (I split this out since the diff is fairly 
> large, but the changes aren't all that exciting: it just moves things 
> from one spot to another, and adds some new index entries.)
> 
> That provides a better spot to explain this new outline-minor-mode 
> support. I also filled in documentation for a few of the existing 
> navigation commands available in Eshell.

Thanks, LGTM.

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Juri Linkov]

>>> This patch adds support for outline-minor-mode to Eshell. When enabled,
>>> this will just add outline buttons to the prompt (as the top level), and
>>> one to the start of the output (as level 2).
> ...
> The main case I'm trying to account for is collapsing the command
> output. I think the most consistent way for Eshell is to make the "heading"
> always be the first line of the output, even if that line is empty. That
> way, the arrow button is always in the same spot (immediately after the
> prompt/input), and if the output had many leading empty lines, you can
> collapse all of them to save space (to be fair, this case probably isn't
> super-common).

Sorry, I still don't understand why do you need two levels.
Is it because in Eshell prompts are often multi-line?

IIUC, with two levels, the case when the output is empty has such problems
that one line will contain two outline headers, that also means
two conflicting margin arrows on the same line?

> For prompts, this isn't as important, since a single-line prompt should
> always have some visible text. For multi-line prompts, it would be possible
> to treat the heading as the first non-empty line, but that would be
> additional work on the Eshell side, and I think we'd still need the
> outline.el changes to handle collapsing the command output. (Improving
> heading-detection for multi-line prompts could always be done in a later
> bug, too.)

So the outline.el changes are required only to handle empty output lines?
That essentially means adding support for two outline headers
on the same line?

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Jim Porter]

On 6/1/2024 11:37 PM, Juri Linkov wrote:
> Sorry, I still don't understand why do you need two levels.
> Is it because in Eshell prompts are often multi-line?

I'm not sure if Eshell prompts are *often* multi-line, but personally I 
use multi-line prompts everywhere I can. Maybe I'm just over-optimizing 
for my own personal preferences here.

> IIUC, with two levels, the case when the output is empty has such problems
> that one line will contain two outline headers, that also means
> two conflicting margin arrows on the same line?

The way I implemented this, this problem wouldn't come up: if the output 
is completely empty, there's no second-level node in the outline for 
that command.

>> For prompts, this isn't as important, since a single-line prompt should
>> always have some visible text. For multi-line prompts, it would be possible
>> to treat the heading as the first non-empty line, but that would be
>> additional work on the Eshell side, and I think we'd still need the
>> outline.el changes to handle collapsing the command output. (Improving
>> heading-detection for multi-line prompts could always be done in a later
>> bug, too.)
> 
> So the outline.el changes are required only to handle empty output lines?
> That essentially means adding support for two outline headers
> on the same line?

To be more precise, the outline.el changes would be required to handle 
the case where a command's output *begins* with one or more newlines. So 
the total output isn't empty, but the first *line* of it is.

In any case, the more I think about this, the more my current patch 
seems like the wrong way to go about this. Even just describing the 
user-facing behavior in all scenarios is pretty complex, so I think it 
might be better to keep it simple and have a single outline level.

That said, for the multi-line prompt case, I wonder if it would make 
sense for outline.el to support multi-line headers. If I could mark the 
entire prompt + command input as a "header", then collapsing it would 
look better: users would still see all of their input in the collapsed 
node. It would look something like so:

   v /home/user/dir
     $ cat some-file.txt
     output
     output
     output

   > /home/user/dir
     $ cat some-file.txt...

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Juri Linkov]

> In any case, the more I think about this, the more my current patch seems
> like the wrong way to go about this. Even just describing the user-facing
> behavior in all scenarios is pretty complex, so I think it might be better
> to keep it simple and have a single outline level.
>
> That said, for the multi-line prompt case, I wonder if it would make sense
> for outline.el to support multi-line headers. If I could mark the entire
> prompt + command input as a "header", then collapsing it would look better:
> users would still see all of their input in the collapsed node.

The multi-line headers have such disadvantage that the outlines
are not compact anymore.  Also multi-line headers might have
more technial issues with displaying an ellipsis at the end.

> It would look something like so:
>
>   v /home/user/dir
>     $ cat some-file.txt
>     output
>     output
>     output
>
>   > /home/user/dir
>     $ cat some-file.txt...

This is a known problem.  Recently I had to add a special handling
in c-ts-mode--outline-predicate to put the outline heading on the
line with the function name instead of the first line with types:

    static void
  v bset_mode_name (struct buffer *b, Lisp_Object val)
    {
      b->mode_name_ = val;
    }

    static void
  v bset_name (struct buffer *b, Lisp_Object val)
    {
      b->name_ = val;
    }

In your case you could do something similar in eshell-outline-search
to put the outline heading on the meaningful line:

    /home/user/dir
  v $ cat some-file.txt
    output
    output
    output

This is not ideal either since the first line belongs to the
previous outline.  This is a known problem too.  For example,
in etc/NEWS, the top +++ is not part of the outline, e.g.

  +++
  *** 'outline-minor-mode' is supported in tree-sitter major modes.
  It can be used in all tree-sitter major modes that set either the
  variable 'treesit-simple-imenu-settings' or 'treesit-outline-predicate'.

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Jim Porter]

On 6/2/2024 11:45 PM, Juri Linkov wrote:
>> In any case, the more I think about this, the more my current patch seems
>> like the wrong way to go about this. Even just describing the user-facing
>> behavior in all scenarios is pretty complex, so I think it might be better
>> to keep it simple and have a single outline level.
>>
>> That said, for the multi-line prompt case, I wonder if it would make sense
>> for outline.el to support multi-line headers. If I could mark the entire
>> prompt + command input as a "header", then collapsing it would look better:
>> users would still see all of their input in the collapsed node.
> 
> The multi-line headers have such disadvantage that the outlines
> are not compact anymore.  Also multi-line headers might have
> more technial issues with displaying an ellipsis at the end.

Given that you mentioned a few other cases where multi-line headers 
might be nice (assuming the number of lines is small), maybe it would 
make sense to see what an implementation of that looks like. I'll see 
about writing a patch for this.

(Another interesting thing I might try is to see if we could provide 
some custom single-line abbreviation for multi-line headers. That would 
let us have compact headers when they're collapsed, even if the "header" 
part is really multiple lines when expanded.)

bug#71284: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell

[Juri Linkov]

>> The multi-line headers have such disadvantage that the outlines
>> are not compact anymore.  Also multi-line headers might have
>> more technial issues with displaying an ellipsis at the end.
>
> Given that you mentioned a few other cases where multi-line headers might
> be nice (assuming the number of lines is small), maybe it would make sense
> to see what an implementation of that looks like. I'll see about writing
> a patch for this.

It would be nice to solve this long-standing problem.

> (Another interesting thing I might try is to see if we could provide some
> custom single-line abbreviation for multi-line headers. That would let us
> have compact headers when they're collapsed, even if the "header" part is
> really multiple lines when expanded.)

Or maybe showing an outline should also show its pre-heading lines
(maybe this would require adding e.g. outline-pre-header-regexp
that matches the first line of such preface lines).

This would be especially useful in cases when there are dozens of
comment lines before the outline heading with the function name
in programming language modes.