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

Previous Next

Full log

Message #11 received at 71284 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 71284 <at> debbugs.gnu.org
Subject: Re: 30.0.50; [PATCH] Add support for outline-minor-mode to Eshell
Date: Fri, 31 May 2024 09:51:28 +0300

> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.