GNU bug report logs -
#71284
30.0.50; [PATCH] Add support for outline-minor-mode to Eshell
Previous Next
To reply to this bug, email your comments to 71284 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Fri, 31 May 2024 05:22:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Jim Porter <jporterbugs <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Fri, 31 May 2024 05:22:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[0001-Add-support-for-outline-minor-mode-in-Eshell.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Fri, 31 May 2024 06:07:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 71284 <at> debbugs.gnu.org (full text, mbox):
> Cc: juri <at> linkov.net
> Date: Thu, 30 May 2024 22:18:44 -0700
> From: Jim Porter <jporterbugs <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Fri, 31 May 2024 07:09:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 71284 <at> debbugs.gnu.org (full text, mbox):
> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Fri, 31 May 2024 19:35:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 71284 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[0001-Add-an-Interaction-chapter-to-the-Eshell-manual.patch (text/plain, attachment)]
[0002-Add-support-for-outline-minor-mode-in-Eshell.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Fri, 31 May 2024 20:04:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 71284 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Sat, 01 Jun 2024 06:01:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 71284 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 31 May 2024 12:33:32 -0700
> Cc: 71284 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Jim Porter <jporterbugs <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Sun, 02 Jun 2024 06:52:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 71284 <at> debbugs.gnu.org (full text, mbox):
>>> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Mon, 03 Jun 2024 04:36:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 71284 <at> debbugs.gnu.org (full text, mbox):
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...
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Mon, 03 Jun 2024 07:18:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 71284 <at> debbugs.gnu.org (full text, mbox):
> 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'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Thu, 06 Jun 2024 02:00:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 71284 <at> debbugs.gnu.org (full text, mbox):
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.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#71284
; Package
emacs
.
(Thu, 06 Jun 2024 06:25:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 71284 <at> debbugs.gnu.org (full text, mbox):
>> 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.
This bug report was last modified 93 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.