GNU bug report logs - #68881
30.0.50; [PATCH] Field properties confuse 'outline-minor-mode'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68881 in the body.
You can then email your comments to 68881 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; [PATCH] Field properties confuse 'outline-minor-mode'
Date: Thu, 1 Feb 2024 15:51:57 -0800

[Message part 1 (text/plain, inline)]

I'd like to add support for 'outline-minor-mode' in Eshell. However, 
Eshell's use of field properties confuses outline.el. Attached is a WIP 
patch + demo code for Eshell that should resolve this.

The main issue was that outline.el uses 'line-beginning-position' and 
friends, which respects field boundaries, but I think we want to avoid 
that for 'outline-minor-mode'. Maybe we could use 'pos-bol' and friends 
instead, but my understanding is that 'line-beginning-position' respects 
display directionality (which we probably want), but 'pos-bol' doesn't.

You can try things out here by starting Eshell and activating 
'outline-minor-mode'. You can also see the problems by applying only the 
Eshell part of the patch.

Any thoughts? Is this the right way to go about this? (Note: I think the 
Eshell side of things will take more work, which I'll address in a later 
bug. However, this should be enough to show off the problems on the 
outline.el side.)

[0001-WIP-Make-outline.el-able-to-handle-buffers-with-fiel.patch (text/plain, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 68881 <at> debbugs.gnu.org
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Wed, 07 Feb 2024 19:37:11 +0200

> I'd like to add support for 'outline-minor-mode' in Eshell. However,
> Eshell's use of field properties confuses outline.el. Attached is a WIP
> patch + demo code for Eshell that should resolve this.
>
> The main issue was that outline.el uses 'line-beginning-position' and
> friends, which respects field boundaries, but I think we want to avoid that
> for 'outline-minor-mode'. Maybe we could use 'pos-bol' and friends instead,
> but my understanding is that 'line-beginning-position' respects display
> directionality (which we probably want), but 'pos-bol' doesn't.

I'm not aware of any cases that would require restricting
outlines to field boundaries.

> You can try things out here by starting Eshell and activating
> 'outline-minor-mode'. You can also see the problems by applying only the
> Eshell part of the patch.

Thanks.  I tried and confirm that your outline.el patch fixes the issue.

> Any thoughts? Is this the right way to go about this? (Note: I think the
> Eshell side of things will take more work, which I'll address in a later
> bug. However, this should be enough to show off the problems on the
> outline.el side.)

There is also one occurrence of 'line-end-position' in outline.el.
Should it have 'inhibit-field-text-motion' as well?

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 68881 <at> debbugs.gnu.org
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Sat, 10 Feb 2024 10:22:16 -0800

[Message part 1 (text/plain, inline)]

On 2/7/2024 9:37 AM, Juri Linkov wrote:
>> The main issue was that outline.el uses 'line-beginning-position' and
>> friends, which respects field boundaries, but I think we want to avoid that
>> for 'outline-minor-mode'. Maybe we could use 'pos-bol' and friends instead,
>> but my understanding is that 'line-beginning-position' respects display
>> directionality (which we probably want), but 'pos-bol' doesn't.
> 
> I'm not aware of any cases that would require restricting
> outlines to field boundaries.

Thanks. After reading the source (generally a good idea), I now see that 
'line-beginning-position' doesn't respect display directionality, so 
there's no reason to use it over 'pos-bol' and friends. I've therefore 
updated the patch to use those, which makes things a bit simpler.

(In any case, I think for RTL, we'd want to use the logical ordering of 
the text anyway, but possibly add the outline buttons on the right side 
of the window instead. That's out of scope for this bug though.)

> There is also one occurrence of 'line-end-position' in outline.el.
> Should it have 'inhibit-field-text-motion' as well?

Thanks for the catch. Updated this as well.

(I also split out the Eshell demonstration patch so that I can drop the 
commit more easily before merging. It's still attached here though in 
case you or anyone else want to try it out.)

[0001-Make-outline.el-ignore-field-properties-in-text.patch (text/plain, attachment)]

[0002-DO-NOT-MERGE-Test-commit-for-previous-patch.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 68881 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#68881: 30.0.50;
 [PATCH] Field properties confuse 'outline-minor-mode'
Date: Sat, 10 Feb 2024 21:23:37 +0200

> Cc: 68881 <at> debbugs.gnu.org
> Date: Sat, 10 Feb 2024 10:22:16 -0800
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 2/7/2024 9:37 AM, Juri Linkov wrote:
> >> The main issue was that outline.el uses 'line-beginning-position' and
> >> friends, which respects field boundaries, but I think we want to avoid that
> >> for 'outline-minor-mode'. Maybe we could use 'pos-bol' and friends instead,
> >> but my understanding is that 'line-beginning-position' respects display
> >> directionality (which we probably want), but 'pos-bol' doesn't.
> > 
> > I'm not aware of any cases that would require restricting
> > outlines to field boundaries.
> 
> Thanks. After reading the source (generally a good idea), I now see that 
> 'line-beginning-position' doesn't respect display directionality, so 
> there's no reason to use it over 'pos-bol' and friends. I've therefore 
> updated the patch to use those, which makes things a bit simpler.
> 
> (In any case, I think for RTL, we'd want to use the logical ordering of 
> the text anyway, but possibly add the outline buttons on the right side 
> of the window instead. That's out of scope for this bug though.)

I don't understand what text directionality has to do with the issue
at hand.  If you elaborate, I could perhaps be of assistance in this
matter.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 68881 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Sat, 10 Feb 2024 13:14:34 -0800

On 2/10/2024 11:23 AM, Eli Zaretskii wrote:
> I don't understand what text directionality has to do with the issue
> at hand.  If you elaborate, I could perhaps be of assistance in this
> matter.

I think nothing in this case. I just wasn't sure initially whether 
'pos-bol' and 'line-beginning-position' handled directionality 
differently. Since they handle it the same way, I think there's no harm 
in using 'pos-bol' in this case.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 68881 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Sun, 11 Feb 2024 08:07:47 +0200

> Date: Sat, 10 Feb 2024 13:14:34 -0800
> Cc: 68881 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 2/10/2024 11:23 AM, Eli Zaretskii wrote:
> > I don't understand what text directionality has to do with the issue
> > at hand.  If you elaborate, I could perhaps be of assistance in this
> > matter.
> 
> I think nothing in this case. I just wasn't sure initially whether 
> 'pos-bol' and 'line-beginning-position' handled directionality 
> differently. Since they handle it the same way, I think there's no harm 
> in using 'pos-bol' in this case.

They both go to the smallest buffer position of the line (modulo the
fields issue).  If an LTR line starts with RTL characters, that buffer
position will not be the leftmost one, but that's all.  (I don't think
we have a function to go to the position that is the leftmost on
display, probably because that was never needed.)

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 68881 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Sat, 10 Feb 2024 23:08:51 -0800

On 2/10/2024 10:07 PM, Eli Zaretskii wrote:
> They both go to the smallest buffer position of the line (modulo the
> fields issue).  If an LTR line starts with RTL characters, that buffer
> position will not be the leftmost one, but that's all.  (I don't think
> we have a function to go to the position that is the leftmost on
> display, probably because that was never needed.)

Looking further in the outline.el code, it seems that RTL support is 
handled elsewhere, so as far as I can tell (as a non-RTL user), 
everything's good on the text directionality front.

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

From: Juri Linkov <juri <at> linkov.net>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 68881 <at> debbugs.gnu.org
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Sun, 11 Feb 2024 19:40:56 +0200

>> There is also one occurrence of 'line-end-position' in outline.el.
>> Should it have 'inhibit-field-text-motion' as well?
>
> Thanks for the catch. Updated this as well.

Here is another catch:

> @@ -725,7 +725,7 @@ outline-insert-heading
>  		(not (string-match (concat "\\`\\(?:" outline-regexp "\\)")
>  				   (concat head " "))))
>        (setq head (concat head " ")))
> -    (unless (bolp) (end-of-line) (newline))
> +    (unless (bolp) (goto-char (pos-bol)) (newline))

This looks like a typo.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 68881 <at> debbugs.gnu.org
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Sun, 11 Feb 2024 10:19:18 -0800

[Message part 1 (text/plain, inline)]

On 2/11/2024 9:40 AM, Juri Linkov wrote:
>> @@ -725,7 +725,7 @@ outline-insert-heading
>>   		(not (string-match (concat "\\`\\(?:" outline-regexp "\\)")
>>   				   (concat head " "))))
>>         (setq head (concat head " ")))
>> -    (unless (bolp) (end-of-line) (newline))
>> +    (unless (bolp) (goto-char (pos-bol)) (newline))
> 
> This looks like a typo.

So it is. Fixed.

[0001-Make-outline.el-ignore-field-properties-in-text.patch (text/plain, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 68881 <at> debbugs.gnu.org
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Mon, 12 Feb 2024 20:25:37 +0200

>>> @@ -725,7 +725,7 @@ outline-insert-heading
>>>   		(not (string-match (concat "\\`\\(?:" outline-regexp "\\)")
>>>   				   (concat head " "))))
>>>         (setq head (concat head " ")))
>>> -    (unless (bolp) (end-of-line) (newline))
>>> +    (unless (bolp) (goto-char (pos-bol)) (newline))
>> This looks like a typo.
>
> So it is. Fixed.

Thanks.  I see no more problems.

Message #37 received at 68881-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 68881-done <at> debbugs.gnu.org
Subject: Re: bug#68881: 30.0.50; [PATCH] Field properties confuse
 'outline-minor-mode'
Date: Mon, 12 Feb 2024 20:03:38 -0800

On 2/12/2024 10:25 AM, Juri Linkov wrote:
>>>> @@ -725,7 +725,7 @@ outline-insert-heading
>>>>    		(not (string-match (concat "\\`\\(?:" outline-regexp "\\)")
>>>>    				   (concat head " "))))
>>>>          (setq head (concat head " ")))
>>>> -    (unless (bolp) (end-of-line) (newline))
>>>> +    (unless (bolp) (goto-char (pos-bol)) (newline))
>>> This looks like a typo.
>>
>> So it is. Fixed.
> 
> Thanks.  I see no more problems.

Thanks. Merged as d570864bebf, so closing this now.

(I'll file a new bug later for adding outline-minor-mode support to 
Eshell once I've worked out all the remaining issues.)

Previous Next

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