GNU bug report logs -
#71284
30.0.50; [PATCH] Add support for outline-minor-mode to Eshell
Previous Next
Full log
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.
This bug report was last modified 123 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.