bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Hi,

This is a follow-up of Org mode bug report
https://orgmode.org/list/8b691a7f-6b62-d573-e5a8-80fac3dc9bc6@vodafonemail.de

Consider the following MWE:

(progn
  (switch-to-buffer (get-buffer-create "*Test*"))
  (erase-buffer)
  (insert "Test case with char before point hidden<point>     :more text:")
  (search-backward ">")
  (forward-char)
  (with-silent-modifications (put-text-property (1- (point)) (point) 'invisible t))
  (let ((indent-tabs-mode nil)) (indent-to 100)))

The text is inserted and then indented as expected when 'invisible
property is not applied:

-------------
Test case with char before point hidden<point>                                                      <point>     :more text:
-------------

However, after un-commenting `put-text-property' line, no indentation is
done "visually" because Finsert_char is called with INHERIT argument Qt.

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.8) of 2023-07-17 built on localhost
Repository revision: 603ac9505d5d0eebf27798620d1826788739fad8
Repository branch: feature/named-lambdas
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Gentoo Linux

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Date: Tue, 18 Jul 2023 07:58:11 +0000
> 
> (progn
>   (switch-to-buffer (get-buffer-create "*Test*"))
>   (erase-buffer)
>   (insert "Test case with char before point hidden<point>     :more text:")
>   (search-backward ">")
>   (forward-char)
>   (with-silent-modifications (put-text-property (1- (point)) (point) 'invisible t))
>   (let ((indent-tabs-mode nil)) (indent-to 100)))
> 
> The text is inserted and then indented as expected when 'invisible
> property is not applied:
> 
> -------------
> Test case with char before point hidden<point>                                                      <point>     :more text:
> -------------
> 
> However, after un-commenting `put-text-property' line, no indentation is
> done "visually" because Finsert_char is called with INHERIT argument Qt.

Isn't that in general TRT?  When you indent text, you _want_ to
inherit the text properties, right?

Whether the inherited properties should also include the invisible
property could be subject to discussion, but it isn't 100% clear to me
that it should always be excluded.

In any case, I think you can bind text-property-default-nonsticky
around the indent-to call to control this, right?  Or use the
rear-sticky text property.

Adding Stefan, in case he has some comments.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> However, after un-commenting `put-text-property' line, no indentation is
>> done "visually" because Finsert_char is called with INHERIT argument Qt.
>
> Isn't that in general TRT?  When you indent text, you _want_ to
> inherit the text properties, right?
>
> Whether the inherited properties should also include the invisible
> property could be subject to discussion, but it isn't 100% clear to me
> that it should always be excluded.

The problem is that inheriting 'invisible (if its value is actually
hiding text) yields to cursor not moving to the target column.

When I read the docstring:

    (indent-to COLUMN &optional MINIMUM)
    
    Documentation
    Indent from point with tabs and spaces until COLUMN is reached.

I expect that (current-column) will become COLUMN after calling
`indent-to'. It is not the case in the described scenario.

More generally, `indent-to' implementation assumes that it is sufficient
to insert "COLUMN - (current-column)" spaces in order to reach COLUMN
(let me ignore tabs for simplicity).

However, it is not true for invisible text and when 'display is
specifying custom space width (and what about :align-to?).

And the actual scenario for Org is even more complex because
Org fontification is what is adding that problematic invisible text
property, leading to race condition between redisplay and indentation.
When I sprinkled (redisplay) calls to different places in Org's
tag alignment code, the results varied depending on where the
(redisplay) call was inserted.

> In any case, I think you can bind text-property-default-nonsticky
> around the indent-to call to control this, right?  Or use the
> rear-sticky text property.

That's what I did to fix things on Org side, but I believe that the
current behaviour is a genuine bug.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 64696@debbugs.gnu.org
> Date: Tue, 18 Jul 2023 12:09:43 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Whether the inherited properties should also include the invisible
> > property could be subject to discussion, but it isn't 100% clear to me
> > that it should always be excluded.
> 
> The problem is that inheriting 'invisible (if its value is actually
> hiding text) yields to cursor not moving to the target column.
> 
> When I read the docstring:
> 
>     (indent-to COLUMN &optional MINIMUM)
>     
>     Documentation
>     Indent from point with tabs and spaces until COLUMN is reached.
> 
> I expect that (current-column) will become COLUMN after calling
> `indent-to'. It is not the case in the described scenario.

If the problem is documentation, we can easily augment the
documentation to match the actual behavior.

So let's first discuss the behavior, not what the documentation says.

> More generally, `indent-to' implementation assumes that it is sufficient
> to insert "COLUMN - (current-column)" spaces in order to reach COLUMN
> (let me ignore tabs for simplicity).

But ignoring the tabs is not a valid "trick" here, because it exactly
means that your interpretation is too naïve: there's more in
indentation to a certain column than meets the eye, precisely because
of stuff like tabs, display properties, composed characters, and other
calamities.

> And the actual scenario for Org is even more complex because
> Org fontification is what is adding that problematic invisible text
> property, leading to race condition between redisplay and indentation.
> When I sprinkled (redisplay) calls to different places in Org's
> tag alignment code, the results varied depending on where the
> (redisplay) call was inserted.

there couldn't be any race condition here, because indent-to is not
run when redisplay runs.  I guess you mean that one could see the
indentation appear, then disappear as result of JIT fontifications, or
not appear at all, depending on the details?

> > In any case, I think you can bind text-property-default-nonsticky
> > around the indent-to call to control this, right?  Or use the
> > rear-sticky text property.
> 
> That's what I did to fix things on Org side, but I believe that the
> current behaviour is a genuine bug.

"Bug" in what sense?  Do you mean that indenting should disregard
properties completely? or just the invisible property? or something
else?  indent-to is a general-purpose primitive, so any change there
will affect gobs of Emacs Lisp programs, and we must specify the
desired behavior in a lot of detail in order to be able to discuss
such changes.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> More generally, `indent-to' implementation assumes that it is sufficient
>> to insert "COLUMN - (current-column)" spaces in order to reach COLUMN
>> (let me ignore tabs for simplicity).
>
> But ignoring the tabs is not a valid "trick" here, because it exactly
> means that your interpretation is too naïve: there's more in
> indentation to a certain column than meets the eye, precisely because
> of stuff like tabs, display properties, composed characters, and other
> calamities.

Ok. More precisely, `indent-to' implementation assumes that it is
sufficient to insert COLUMN/tab_width - (current-column))/tab_width tabs
and then add (COLUMN - (current-column) - N_tabs * tab_width) spaces.
(when indent-tabs-mode is active)

Still not taking into account display and invisible properties. Just
assuming that width of the inserted tabs and spaces is fixed.

At the same time, `current_column' does take into account display
properties. And thus we have one part of `indent-to' considering
invisible and display properties, while other part ignoring them.

>> And the actual scenario for Org is even more complex because
>> Org fontification is what is adding that problematic invisible text
>> property, leading to race condition between redisplay and indentation.
>> When I sprinkled (redisplay) calls to different places in Org's
>> tag alignment code, the results varied depending on where the
>> (redisplay) call was inserted.
>
> there couldn't be any race condition here, because indent-to is not
> run when redisplay runs.  I guess you mean that one could see the
> indentation appear, then disappear as result of JIT fontifications, or
> not appear at all, depending on the details?

No. I mean that (current-column) may change after fontification is
applied. We had a similar issue recently in
https://list.orgmode.org/als4q47p4dt4iva4s73sihe75qjdz4ni554h55e63o6izogzst@vs3olesw6da3/T/#m8d85b510def4163f8401988aaa44bf935027745f

Though this part may or may not be considered a bug. I am more concerned
about 'invisible property.

>> That's what I did to fix things on Org side, but I believe that the
>> current behaviour is a genuine bug.
>
> "Bug" in what sense?

In a sense that `indent-to' does not do what it is documented to do.
So, either documentation should be corrected or the implementation
should be changed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> However, after un-commenting `put-text-property' line, no indentation is
>>> done "visually" because Finsert_char is called with INHERIT argument Qt.
>> Isn't that in general TRT?  When you indent text, you _want_ to
>> inherit the text properties, right?
>> Whether the inherited properties should also include the invisible
>> property could be subject to discussion, but it isn't 100% clear to me
>> that it should always be excluded.
>
> The problem is that inheriting 'invisible (if its value is actually
> hiding text) yields to cursor not moving to the target column.
> When I read the docstring:
[...]
>     Indent from point with tabs and spaces until COLUMN is reached.

That's a good point.

Could you give a bit of background about why you use set an `invisible`
property that is inherited when inserting chars after it?

I mean I understand it's the default behavior of the `invisible`
property, but when making a piece of text invisible, it's common
that we arrange for that invisible property not to leak to text inserted
after it (either by setting the marker type of the overlay's end
or via the `rear-nonsticky` text-property).
This is useful for when the users goes and types right after the
invisible text, otherwise their text becomes invisible.

Of course, the command loop also tries to move point away from such
positions, so it can be useful not to use `rear-nonsticky`, so as to
"force" point to go before the invisible text rather than after.  But in
that case I'd expect that you'd also want to call `indent-to` with point
before the invisible text rather than after.

>> In any case, I think you can bind text-property-default-nonsticky
>> around the indent-to call to control this, right?  Or use the
>> rear-sticky text property.
> That's what I did to fix things on Org side, but I believe that the
> current behaviour is a genuine bug.

I think the only non-ugly fix would be to make `indent-to` call
`insert-char` without the `inherit` argument.

The history of this code is simple: `indent-to` was changed to pass the
`inherit` arg back in Aug 1994 at the same time `insert-char` got its new
`inherit` argument (see commits 6d1bd1a58117 and e2eeabbbce46),
i.e. quite early on, around the time text-properties were added.

FWIW, I can't see any good reason to inherit properties here, other than
"we've always done that", but I'd be surprised if making this change
won't affect some other use elsewhere :-(


        Stefan

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Tue, 18 Jul 2023 13:25:55 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > "Bug" in what sense?
> 
> In a sense that `indent-to' does not do what it is documented to do.
> So, either documentation should be corrected or the implementation
> should be changed.

I was actually trying to understand what alternative behavior you'd
like to see.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  64696@debbugs.gnu.org
> Date: Tue, 18 Jul 2023 10:15:48 -0400
> 
> FWIW, I can't see any good reason to inherit properties here, other than
> "we've always done that", but I'd be surprised if making this change
> won't affect some other use elsewhere :-(

??? That's a non-starter, from where I stand.  If you really think it
can be useful for the inserted whitespace to lose _all_ of the
properties, I can suggest a new optional argument to do that.  Would
that be good enough?

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> I was actually trying to understand what alternative behavior you'd
> like to see.

I am not sure, because I do not fully understand how the new
implementation of `current-column' works in the wild.

My general understanding of the indentation is that it works "visually",
so that indentation makes things look aligned to graphical column in
the displayed Emacs windows.

However, I do not fully understand what happens if we try to indent
fully hidden text (like inside Org's folds). See my previous bug
reports: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56837 and
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58791

You mentioned in the past that indenting invisible text does not make
sense, although it does in Org, and things did not break spectacularly
after bug#58791 was fixed.

The overall design of `indent-to' is unfortunately elusive for me.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Tue, 18 Jul 2023 16:25:31 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I was actually trying to understand what alternative behavior you'd
> > like to see.
> 
> I am not sure, because I do not fully understand how the new
> implementation of `current-column' works in the wild.
> 
> My general understanding of the indentation is that it works "visually",
> so that indentation makes things look aligned to graphical column in
> the displayed Emacs windows.

It tries, yes.

> However, I do not fully understand what happens if we try to indent
> fully hidden text (like inside Org's folds). See my previous bug
> reports: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56837 and
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58791

Invisible text is skipped by current-column.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> FWIW, I can't see any good reason to inherit properties here, other than
>> "we've always done that", but I'd be surprised if making this change
>> won't affect some other use elsewhere :-(
>
> ??? That's a non-starter, from where I stand.

I formulated it differently, but you're just agreeing with the "but ..."
quoted above :-)

> If you really think it can be useful for the inserted whitespace to
> lose _all_ of the properties,

Yes, I think if we could go back in 1994 and not pass that arg back
then, it would have been a better choice and wouldn't have introduced
any significant problem.

> I can suggest a new optional argument to do that.  Would that be
> good enough?

It might indeed be the best we can do now.  It's probably going to be
ugly to get that argument from where we know we need it through other
functions to `indent-to` :-(

But before we go through that, I'd still like to hear the general
context where the problem occurs in Org (e.g. why wasn't
`read-nonsticky` used).


        Stefan

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> However, I do not fully understand what happens if we try to indent
>> fully hidden text (like inside Org's folds). See my previous bug
>> reports: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56837 and
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58791
>
> Invisible text is skipped by current-column.

I am not sure if I understand what you mean by "skipped".
Consider the following file in outline-mode:

-----
<point>
* This
word word
-----

I tried the following command with M-:

(progn
  (visible-mode -1)
  (search-forward "word")
  (message "1:: current-column = %d" (current-column))
  (redisplay)
  (message "2:: current-column = %d" (current-column))
  (indent-to 50)
  (message "3:: current-column = %d" (current-column))
  (redisplay)
  (message "4:: current-column = %d" (current-column))
  (visible-mode +1)
  (message "5:: current-column = %d" (current-column))
  (redisplay)
  (message "6:: current-column = %d" (current-column))
  (visible-mode -1))

First, I left the heading unfolded:

1:: current-column = 4
2:: current-column = 4
3:: current-column = 50
4:: current-column = 50
5:: current-column = 50
6:: current-column = 50

Then, I undid the indent, folded the heading, and repeated

1:: current-column = 4 <==== same despite text being invisible via overlay
2:: current-column = 4
3:: current-column = 50
4:: current-column = 50
5:: current-column = 50
6:: current-column = 50

Then, I undid the indent again, and applied 'invisible text property to
the heading text

1:: current-column = 0 <==== now, `current-column' ignores the invisible text
2:: current-column = 0
3:: current-column = 50 <=== This presumably inserts more tabs/spaces, but not!
4:: current-column = 0  <=== redisplay changed `current-column' ??
5:: current-column = 50 <=== indeed, the number of spaces is the same as visible text
6:: current-column = 50

Then, I repeated again, but applied 'rear-nonsticky (invisible) text
property as well

1:: current-column = 0
2:: current-column = 0
3:: current-column = 50
4:: current-column = 50
5:: current-column = 50
6:: current-column = 50

I cannot make much sense of the above results.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Could you give a bit of background about why you use set an `invisible`
> property that is inherited when inserting chars after it?
>
> I mean I understand it's the default behavior of the `invisible`
> property, but when making a piece of text invisible, it's common
> that we arrange for that invisible property not to leak to text inserted
> after it (either by setting the marker type of the overlay's end
> or via the `rear-nonsticky` text-property).
> This is useful for when the users goes and types right after the
> invisible text, otherwise their text becomes invisible.

Because Org sets and clears invisible property manually during fontification.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> FWIW, I can't see any good reason to inherit properties here, other than
>>> "we've always done that", but I'd be surprised if making this change
>>> won't affect some other use elsewhere :-(
>>
>> ??? That's a non-starter, from where I stand.
>
> I formulated it differently, but you're just agreeing with the "but ..."
> quoted above :-)

Note that unlike `indent-to', `insert' does not inherit properties.
Not to say that changing `indent-to' is safe, but there is slight
historical inconsistency here.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Wed, 19 Jul 2023 08:30:28 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> However, I do not fully understand what happens if we try to indent
> >> fully hidden text (like inside Org's folds). See my previous bug
> >> reports: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56837 and
> >> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58791
> >
> > Invisible text is skipped by current-column.
> 
> I am not sure if I understand what you mean by "skipped".

"Skipped" means that the invisible text is not counted as occupying
columns on display.  It is skipped without incrementing the column
count.  See this part of scan_for_column:

  /* Scan forward to the target position.  */
  while (scan < end)
    {
      int c;

      /* Occasionally we may need to skip invisible text.  */
      while (scan == next_boundary)
	{
	  ptrdiff_t old_scan = scan;
	  /* This updates NEXT_BOUNDARY to the next place
	     where we might need to skip more invisible text.  */
	  scan = skip_invisible (scan, &next_boundary, end, Qnil);

And note that I was only talking about current-column (and
move-to-column, which AFAIR behaves the same).

> Consider the following file in outline-mode:
> [...]

Please make it easier for me to follow the questions you are asking
and provide helpful responses by (a) making the recipes as simple as
possible, and (b) by describing them in full detail.  "Folded",
"unfolded heading", "applied invisible text property", etc. -- all
those might be crystal clear to you, but they aren't so to me, and
require me to guess (and err) what exactly did you do and how.

Please understand: I have very limited time to invest in answering
questions about how the Emacs internals work and why.  I have too
little free time and too much to do, day in and day out, just to keep
this project moving.  So please have mercy on me, and help me answer
your questions (a lot of them lately, btw), in a useful manner,
without spending an inordinate amount of time.  Each such question
usually requires looking up stuff in the code, describing it to you in
enough detail for you to understand, sometimes stepping through the
code in a debugger to make sure the code really works like I think it
does, etc.  Add to this the need to understand what you mean by all
those undisclosed details, and the burden becomes unbearable.

Please!

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 64696@debbugs.gnu.org
> Date: Wed, 19 Jul 2023 08:42:45 +0000
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> >>> FWIW, I can't see any good reason to inherit properties here, other than
> >>> "we've always done that", but I'd be surprised if making this change
> >>> won't affect some other use elsewhere :-(
> >>
> >> ??? That's a non-starter, from where I stand.
> >
> > I formulated it differently, but you're just agreeing with the "but ..."
> > quoted above :-)
> 
> Note that unlike `indent-to', `insert' does not inherit properties.
> Not to say that changing `indent-to' is safe, but there is slight
> historical inconsistency here.

I don't see any inconsistency.  'insert' is a much more
general-purpose primitive than indent-to.  It would be dead wrong for
each insertion to inherit the properties.

indent-to is different: when you indent text, you supposed want the
whitespace to use the same face, for example.  Think about it.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Could you give a bit of background about why you use set an `invisible`
>> property that is inherited when inserting chars after it?
>>
>> I mean I understand it's the default behavior of the `invisible`
>> property, but when making a piece of text invisible, it's common
>> that we arrange for that invisible property not to leak to text inserted
>> after it (either by setting the marker type of the overlay's end
>> or via the `rear-nonsticky` text-property).
>> This is useful for when the users goes and types right after the
>> invisible text, otherwise their text becomes invisible.
>
> Because Org sets and clears invisible property manually during fontification.

I don't see how that explains why you chose to set an `invisible`
property that is inherited when inserting chars after it.


        Stefan

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't see any inconsistency.  'insert' is a much more
> general-purpose primitive than indent-to.  It would be dead wrong for
> each insertion to inherit the properties.
>
> indent-to is different: when you indent text, you supposed want the
> whitespace to use the same face, for example.  Think about it.

Not necessarily. Sometimes, we just use it for plain text alignment.
Like in block comments or Org tables.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Wed, 19 Jul 2023 14:25:48 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I don't see any inconsistency.  'insert' is a much more
> > general-purpose primitive than indent-to.  It would be dead wrong for
> > each insertion to inherit the properties.
> >
> > indent-to is different: when you indent text, you supposed want the
> > whitespace to use the same face, for example.  Think about it.
> 
> Not necessarily. Sometimes, we just use it for plain text alignment.
> Like in block comments or Org tables.

I can understand "sometimes".  My point was that it isn't "always" nor
even "almost always".  IOW, the fact that indent-to inherits is not
arbitrary.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Invisible text is skipped by current-column.
>> 
>> I am not sure if I understand what you mean by "skipped".
>
> "Skipped" means that the invisible text is not counted as occupying
> columns on display.  It is skipped without incrementing the column
> count.  See this part of scan_for_column:

In my tests, invisible overlay property does not appear to affect things.

>> Consider the following file in outline-mode:
>> [...]
>
> Please make it easier for me to follow the questions you are asking
> and provide helpful responses by (a) making the recipes as simple as
> possible, and (b) by describing them in full detail.  "Folded",
> "unfolded heading", "applied invisible text property", etc. -- all
> those might be crystal clear to you, but they aren't so to me, and
> require me to guess (and err) what exactly did you do and how.

Sure.
You can do M-x yant/full-test after evaluating the code below

(defun yant/test-indent-columns ()
  (visible-mode -1)
  (search-forward "word")
  (warn "Moved point after first 'word'")
  (warn "1:: current-column = %d" (current-column))
  (redisplay)
  (warn "redisplayed")
  (warn "2:: current-column = %d" (current-column))
  (indent-to 50)
  (warn "Indented to column 50")
  (warn "3:: current-column = %d" (current-column))
  (redisplay)
  (warn "redisplayed")
  (warn "4:: current-column = %d" (current-column))
  (visible-mode +1)
  (warn "Enabled visible mode")
  (warn "5:: current-column = %d" (current-column))
  (redisplay)
  (warn "redisplayed")
  (warn "6:: current-column = %d" (current-column))
  (visible-mode -1)
  (read-char "Press any key")
  (kill-buffer))

(defun yant/test-init-buffer ()
  (switch-to-buffer (get-buffer-create "*Test*"))
  (delete-other-windows)
  (unless (eq major-mode 'outline-mode) (outline-mode))
  (erase-buffer)
  (insert "\n")
  (insert "* Test\n")
  (insert "word word\n")
  (goto-char (point-min)))

(defun yant/full-test ()
  (interactive)
  (when (get-buffer "*Warnings*")
    (with-current-buffer "*Warnings*" (with-silent-modifications (erase-buffer))))
  (yant/test-init-buffer)
  (warn "We are going to indent second 'word' to column 50 and check `current-column'.")
  (outline-show-all)
  (warn "Test #1:: Everything visible.")
  (read-char "Press any key to start the test")
  (yant/test-indent-columns)
  (yant/test-init-buffer)
  (warn "Test #2:: 'word' is inside folded heading (hidden using overlays).")
  (outline-cycle-buffer)
  (read-char "Press any key to start the test")
  (yant/test-indent-columns)
  (yant/test-init-buffer)
  (warn "Test #3:: 'word' is hidden applying 'invisible text property to heading body.")
  (outline-show-all)
  (save-excursion
    (search-forward "Test")
    (with-silent-modifications (put-text-property (point) (point-max) 'invisible t)))
  (read-char "Press any key to start the test")
  (yant/test-indent-columns)
  (yant/test-init-buffer)
  (warn "Test #4:: 'word' is hidden applying non-sticky 'invisible text property to heading body.")
  (outline-show-all)
  (save-excursion
    (search-forward "Test")
    (with-silent-modifications (put-text-property (point) (point-max) 'invisible t))
    (with-silent-modifications (put-text-property (point) (point-max) 'rear-nonsticky '(invisible))))
  (read-char "Press any key to continue")
  (yant/test-indent-columns))


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> (defun yant/test-init-buffer ()
>   (switch-to-buffer (get-buffer-create "*Test*"))
>   (delete-other-windows)
>   (unless (eq major-mode 'outline-mode) (outline-mode))
>   (erase-buffer)
>   (insert "\n")
>   (insert "* Test\n")
>   (insert "word word\n")
>   (goto-char (point-min)))

And results also somehow depend on `indent-tabs-mode'.
If I try

(defun yant/test-init-buffer ()
  (switch-to-buffer (get-buffer-create "*Test*"))
  (delete-other-windows)
  (unless (eq major-mode 'outline-mode) (outline-mode))
  (erase-buffer)
  (indent-tabs-mode -1) ;; <<< do not indent using tabs
  (insert "\n")
  (insert "* Test\n")
  (insert "word word\n")
  (goto-char (point-min)))

With `indent-tabs-mode' disabled, running the test yields different
results. (AFAIU, it should not).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Thu, 20 Jul 2023 09:10:02 +0000
> 
> You can do M-x yant/full-test after evaluating the code below

And what are the problems you see with current-column in this example?
(Let's leave indent-to alone for now.)

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Fri, 21 Jul 2023 08:32:18 +0000
> 
> Ihor Radchenko <yantar92@posteo.net> writes:
> 
> And results also somehow depend on `indent-tabs-mode'.
> If I try
> 
> (defun yant/test-init-buffer ()
>   (switch-to-buffer (get-buffer-create "*Test*"))
>   (delete-other-windows)
>   (unless (eq major-mode 'outline-mode) (outline-mode))
>   (erase-buffer)
>   (indent-tabs-mode -1) ;; <<< do not indent using tabs
>   (insert "\n")
>   (insert "* Test\n")
>   (insert "word word\n")
>   (goto-char (point-min)))
> 
> With `indent-tabs-mode' disabled, running the test yields different
> results. (AFAIU, it should not).

Is this about indent-to or about current-column?  I'd like to see
first if there are any problems in current-column.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@posteo.net>
>> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
>> Date: Thu, 20 Jul 2023 09:10:02 +0000
>> 
>> You can do M-x yant/full-test after evaluating the code below
>
> And what are the problems you see with current-column in this example?
> (Let's leave indent-to alone for now.)

⛔ Warning (emacs): Test #1:: Everything visible.
⛔ Warning (emacs): Moved point after first ’word’
⛔ Warning (emacs): 1:: current-column = 4

Test #1 is expected - everything is visible, we are indeed at column 4.

⛔ Warning (emacs): Test #2:: ’word’ is inside folded heading (hidden using overlays).
⛔ Warning (emacs): Moved point after first ’word’
⛔ Warning (emacs): 1:: current-column = 4

Test #2 is unexpected - we are inside invisible region, but
current-column reports as if everything were visible.

⛔ Warning (emacs): Test #3:: ’word’ is hidden applying ’invisible text property to heading body.
⛔ Warning (emacs): Moved point after first ’word’
⛔ Warning (emacs): 1:: current-column = 0

Test #3 follows your previous explanation that current-column ignores
invisible text.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> With `indent-tabs-mode' disabled, running the test yields different
>> results. (AFAIU, it should not).
>
> Is this about indent-to or about current-column?  I'd like to see
> first if there are any problems in current-column.

Ok.

Test #3 with and without indent-tabs-mode are not the same:

(indent-tabs-mode is nil)
⛔ Warning (emacs): Test #3:: ’word’ is hidden applying ’invisible text property to heading body.
⛔ Warning (emacs): Moved point after first ’word’
⛔ Warning (emacs): 1:: current-column = 0
⛔ Warning (emacs): Indented to column 50
⛔ Warning (emacs): 3:: current-column = 0
⛔ Warning (emacs): Enabled visible mode
⛔ Warning (emacs): 5:: current-column = 54

(indent-tabs-mode is t)
⛔ Warning (emacs): Test #3:: ’word’ is hidden applying ’invisible text property to heading body.
⛔ Warning (emacs): Moved point after first ’word’
⛔ Warning (emacs): 1:: current-column = 0
⛔ Warning (emacs): Indented to column 50
⛔ Warning (emacs): 3:: current-column = 0
⛔ Warning (emacs): Enabled visible mode
⛔ Warning (emacs): 5:: current-column = 50

Note how the last current-column value is different.
It may be something about current-column or about indent-to.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sat, 22 Jul 2023 07:03:41 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> You can do M-x yant/full-test after evaluating the code below
> >
> > And what are the problems you see with current-column in this example?
> > (Let's leave indent-to alone for now.)
> 
> ⛔ Warning (emacs): Test #1:: Everything visible.
> ⛔ Warning (emacs): Moved point after first ’word’
> ⛔ Warning (emacs): 1:: current-column = 4
> 
> Test #1 is expected - everything is visible, we are indeed at column 4.
> 
> ⛔ Warning (emacs): Test #2:: ’word’ is inside folded heading (hidden using overlays).
> ⛔ Warning (emacs): Moved point after first ’word’
> ⛔ Warning (emacs): 1:: current-column = 4
> 
> Test #2 is unexpected - we are inside invisible region, but
> current-column reports as if everything were visible.

current-column produces incorrect results when the newline before the
current line is invisible.  It always starts from the beginning of the
current physical line, even if that is in invisible text.

We could teach current-column about invisible newlines, see the patch
below.  But I'm not sure this is justified, nor whether it won't break
something.  The patch below also has a disadvantage that it will still
behave as before for a buffer that is not displayed in any window; if
we want that to be fixed as well, the changes will need to be more
extensive.  (Basically, we will need to write a non-display version of
back_to_previous_visible_line_start.)

With the patch below, Test #2 shows "current-column = 6", which is
correct, since the cursor is shown after "* Test", with all the rest
invisible.

If we think this kind of change is a good idea (Stefan?), patches to
make the below work without employing display code (which needs the
window) will be welcome.

diff --git a/src/indent.c b/src/indent.c
index eda85f2..1357fd2 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -563,11 +563,35 @@ scan_for_column (ptrdiff_t *endpos, EMACS_INT *goalcol,
   ptrdiff_t end = endpos ? *endpos : PT;
   ptrdiff_t scan, scan_byte, next_boundary, prev_pos, prev_bpos;
 
-  scan = find_newline (PT, PT_BYTE, BEGV, BEGV_BYTE, -1, NULL, &scan_byte, 1);
-
   window = Fget_buffer_window (Fcurrent_buffer (), Qnil);
   w = ! NILP (window) ? XWINDOW (window) : NULL;
 
+  int prevbyte = 0;
+  Lisp_Object prop = Qnil;
+  if (PT > BEGV && w)
+    {
+      prevbyte = FETCH_BYTE (PT_BYTE - 1);
+      prop  = Fget_char_property (make_fixnum (PT - 1), Qinvisible, window);
+    }
+  if (w && !(prevbyte == '\n' && TEXT_PROP_MEANS_INVISIBLE (prop) == 0))
+    {
+      struct it it;
+      struct text_pos pt;
+      specpdl_ref count = SPECPDL_INDEX ();
+      void *itdata = bidi_shelve_cache ();
+      SET_TEXT_POS (pt, PT, PT_BYTE);
+      record_unwind_protect_void (unwind_display_working_on_window);
+      display_working_on_window_p = true;
+      start_display (&it, w, pt);
+      reseat_at_previous_visible_line_start (&it);
+      scan = IT_CHARPOS (it);
+      scan_byte = IT_BYTEPOS (it);
+      bidi_unshelve_cache (itdata, 0);
+      unbind_to (count, Qnil);
+    }
+  else
+    scan = find_newline (PT, PT_BYTE, BEGV, BEGV_BYTE, -1, NULL, &scan_byte, 1);
+
   if (current_buffer->long_line_optimizations_p)
     {
       bool lines_truncated = false;

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sat, 22 Jul 2023 07:09:25 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> With `indent-tabs-mode' disabled, running the test yields different
> >> results. (AFAIU, it should not).
> >
> > Is this about indent-to or about current-column?  I'd like to see
> > first if there are any problems in current-column.
> 
> Ok.
> 
> Test #3 with and without indent-tabs-mode are not the same:
> 
> (indent-tabs-mode is nil)
> ⛔ Warning (emacs): Test #3:: ’word’ is hidden applying ’invisible text property to heading body.
> ⛔ Warning (emacs): Moved point after first ’word’
> ⛔ Warning (emacs): 1:: current-column = 0
> ⛔ Warning (emacs): Indented to column 50
> ⛔ Warning (emacs): 3:: current-column = 0
> ⛔ Warning (emacs): Enabled visible mode
> ⛔ Warning (emacs): 5:: current-column = 54
> 
> (indent-tabs-mode is t)
> ⛔ Warning (emacs): Test #3:: ’word’ is hidden applying ’invisible text property to heading body.
> ⛔ Warning (emacs): Moved point after first ’word’
> ⛔ Warning (emacs): 1:: current-column = 0
> ⛔ Warning (emacs): Indented to column 50
> ⛔ Warning (emacs): 3:: current-column = 0
> ⛔ Warning (emacs): Enabled visible mode
> ⛔ Warning (emacs): 5:: current-column = 50
> 
> Note how the last current-column value is different.
> It may be something about current-column or about indent-to.

(With the patched current-column, I get 48 and 50.)

It's neither.  If you think about this, you will realize that
something like this is expected: current-column will always return
different results when you remove the invisibility properties,
depending on whether tabs were used for indentation, because the width
of a tab is not constant, it depends on where on display it is
located.

In this case, the "real" column after the first "word" is 4, but when
there are invisible properties, its column is 6 (with the patched
current-column).  So indent-to needs to insert the equivalent of 44
columns to get to column 50.  With indent-tabs-mode nil it actually
inserts 44 spaces, so when the invisibility is removed, we get
current-column = 4 + 44 = 48.  When indent-tabs-mode is t, indent-to
decides to insert 6 tabs and 2 spaces, so you get a different result
when you remove the invisibility.

Bottom line: indentation with invisible text is tricky.  (Who was the
genius who decided that using invisibility in Org is a good idea??)

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> ⛔ Warning (emacs): Test #2:: ’word’ is inside folded heading (hidden using overlays).
> ⛔ Warning (emacs): Moved point after first ’word’
> ⛔ Warning (emacs): 1:: current-column = 4
>
> Test #2 is unexpected - we are inside invisible region, but
> current-column reports as if everything were visible.

Side note: `current-column` treats invisible text differently
depending on whether it's replaced by ellipses or not.

This was done so that reindenting a region or a sexp doesn't go haywire
when part of the region is currently hidden by code folding.

I now think that relying on "ellipses or not" was not a great idea
(there are ellipses which `current-column` should count as the width of
the ellipsis rather than the width of the hidden text, e.g. when
ellipses are used to truncate cells in a table).


        Stefan

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> ⛔ Warning (emacs): Test #2:: ’word’ is inside folded heading (hidden using overlays).
>> ⛔ Warning (emacs): Moved point after first ’word’
>> ⛔ Warning (emacs): 1:: current-column = 4
>> 
>> Test #2 is unexpected - we are inside invisible region, but
>> current-column reports as if everything were visible.
>
> current-column produces incorrect results when the newline before the
> current line is invisible.  It always starts from the beginning of the
> current physical line, even if that is in invisible text.

Then why does test #3 produce current-column = 0?
The newline before current line is also invisible there.

> We could teach current-column about invisible newlines, see the patch
> below.  But I'm not sure this is justified, nor whether it won't break
> something.  The patch below also has a disadvantage that it will still
> behave as before for a buffer that is not displayed in any window; if
> we want that to be fixed as well, the changes will need to be more
> extensive.  (Basically, we will need to write a non-display version of
> back_to_previous_visible_line_start.)
>
> With the patch below, Test #2 shows "current-column = 6", which is
> correct, since the cursor is shown after "* Test", with all the rest
> invisible.

This will definitely break indentation code.

Look at `indent-line-to':

  (beginning-of-line 1)
  (skip-chars-forward " \t")
  (let ((cur-col (current-column)))
<...>
((> cur-col column) ; too far right (after tab?)
           (delete-region (progn (move-to-column column t) (point))
                          ;; The `move-to-column' call may replace
                          ;; tabs with spaces, so we can't reuse the
                          ;; previous start point.
                          (progn (beginning-of-line 1)
                                 (skip-chars-forward " \t")
                                 (point))))

In the Test #2 situation, if `current-column' returns 6 and we have
* Test
 <point>word

(indent-line-to 3)

we will go into (> cur-col column) branch and
(delete-region <line end position> <first non-whitespace after bol>)

In other words, `indent-line-to' with delete "word".

I am pretty sure that it is not the only breakage.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> Bottom line: indentation with invisible text is tricky.  (Who was the
> genius who decided that using invisibility in Org is a good idea??)

You mean 'invisible property? Or 'invisible overlays?
The latter is inherited from outline-mode.
The former is me, because performance, and because there were no
obvious hard blockers.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sat, 22 Jul 2023 14:05:00 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Test #2 is unexpected - we are inside invisible region, but
> >> current-column reports as if everything were visible.
> >
> > current-column produces incorrect results when the newline before the
> > current line is invisible.  It always starts from the beginning of the
> > current physical line, even if that is in invisible text.
> 
> Then why does test #3 produce current-column = 0?
> The newline before current line is also invisible there.

I'm guessing it's sheer luck or something.  Bugs are like that: they
don't always behave consistently if you change the conditions
slightly.

I didn't look at that case, and frankly, I'm not thrilled to do that.
The code starts by going to the previous newline, and doesn't pay
attention to whether the newline is itself invisible.  Thereafter, it
looks only for _changes_ in the invisible property, so it will not
consider a stretch of text starting from that newline as invisible.
Given this strategy, it should be clear that the result cannot be
correct in general when the newline at BOL is invisible; all the rest
are details that are not really relevant, and spending time on
understanding exactly what happens in this or that particular case is
from my POV a waste of time.

> > We could teach current-column about invisible newlines, see the patch
> > below.  But I'm not sure this is justified, nor whether it won't break
> > something.  The patch below also has a disadvantage that it will still
> > behave as before for a buffer that is not displayed in any window; if
> > we want that to be fixed as well, the changes will need to be more
> > extensive.  (Basically, we will need to write a non-display version of
> > back_to_previous_visible_line_start.)
> >
> > With the patch below, Test #2 shows "current-column = 6", which is
> > correct, since the cursor is shown after "* Test", with all the rest
> > invisible.
> 
> This will definitely break indentation code.

But is correct, don't you agree?

> Look at `indent-line-to':
> 
>   (beginning-of-line 1)
>   (skip-chars-forward " \t")
>   (let ((cur-col (current-column)))
> <...>
> ((> cur-col column) ; too far right (after tab?)
>            (delete-region (progn (move-to-column column t) (point))
>                           ;; The `move-to-column' call may replace
>                           ;; tabs with spaces, so we can't reuse the
>                           ;; previous start point.
>                           (progn (beginning-of-line 1)
>                                  (skip-chars-forward " \t")
>                                  (point))))

This obviously assumes the text is not invisible.

> I am pretty sure that it is not the only breakage.

I don't insist in making that change.  Quite the opposite, actually.
I also expect it to break gobs of indentation code where invisible
text is involved.  Indentation code should probably temporarily remove
the invisibility spec, while indenting, or something.

Once again, the indentation feature was designed to work on PL source
text and on human-readable text files where the user types text that
is completely, or almost completely, visible.  It was not designed for
Outline and similar modes which hide large portions of the buffer
text.  So it is a small wonder that it doesn't work well in those
cases.

The main motivation to fix scan_for_column to consider more visual
effects was so vertical cursor motion works as expected when large
portions of text are hidden.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sat, 22 Jul 2023 14:10:03 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Bottom line: indentation with invisible text is tricky.  (Who was the
> > genius who decided that using invisibility in Org is a good idea??)
> 
> You mean 'invisible property? Or 'invisible overlays?

I mean both.  The problems happen with both, and current-column treats
both the same, as you can see in skip_invisible (which is also used by
the display code, so the results should be the same as on display).

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> > With the patch below, Test #2 shows "current-column = 6", which is
>> > correct, since the cursor is shown after "* Test", with all the rest
>> > invisible.
>> 
>> This will definitely break indentation code.
>
> But is correct, don't you agree?

Depends on the usage of `current-column' and design of Emacs
indentation.

On one hand, Emacs' indentation is nicely aligning text as is it
actually displayed in the buffer, taking into account all kinds of bells
and whistles that alter the displayed text width.

On the other hand, such visual indentation is not always good. Visuals
present in one Emacs config may not be enabled in another config. And
code/text nicely aligned on one machine will suddenly look ugly on
other. For example, see
http://endlessparentheses.com/using-prettify-symbols-in-clojure-and-elisp-without-breaking-indentation.html

In Org mode, visual indentation is also not necessarily good thing:

1. Org mode is generally aiming for the produced Org files to be
   readable as unfontified plain text. So, quirks related to visual
   indentation generally tend to mess things up.
2. Org mode uses indentation as part of the syntax. I had to get rid of
   using `current-column' and calculated "true textual indentation"
   manually to avoid breakage after `current-column' started to take
   into account invisibility. (bug#56837)

>> I am pretty sure that it is not the only breakage.
>
> I don't insist in making that change.  Quite the opposite, actually.
> I also expect it to break gobs of indentation code where invisible
> text is involved.  Indentation code should probably temporarily remove
> the invisibility spec, while indenting, or something.

That would make sense, yes.

> The main motivation to fix scan_for_column to consider more visual
> effects was so vertical cursor motion works as expected when large
> portions of text are hidden.

What about having something like `current-visual-column' that will be
used when we really need to examine which display column the cursor is
it, accounting for all the display-affecting properties?

Or maybe even have `use-visual-columns' variable that will modify how
`current-column' behaves ('visual, 'textual, 'visual-ignore-invisible,
etc)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sun, 23 Jul 2023 07:30:34 +0000
> 
> On one hand, Emacs' indentation is nicely aligning text as is it
> actually displayed in the buffer, taking into account all kinds of bells
> and whistles that alter the displayed text width.
> 
> On the other hand, such visual indentation is not always good. Visuals
> present in one Emacs config may not be enabled in another config. And
> code/text nicely aligned on one machine will suddenly look ugly on
> other. For example, see
> http://endlessparentheses.com/using-prettify-symbols-in-clojure-and-elisp-without-breaking-indentation.html

Programs that want "indentation" that only counts codepoints (which is
really a rare situation, see below) need to disable all kinds of Emacs
features, otherwise they will not get what they want.

> In Org mode, visual indentation is also not necessarily good thing:
> 
> 1. Org mode is generally aiming for the produced Org files to be
>    readable as unfontified plain text. So, quirks related to visual
>    indentation generally tend to mess things up.

"Unfontified"?  AFAIK, Org files do use fontifications, don't they?
So what do you mean by "unfontified plain text" here?  But that's an
aside, feel free to ignore.

> 2. Org mode uses indentation as part of the syntax. I had to get rid of
>    using `current-column' and calculated "true textual indentation"
>    manually to avoid breakage after `current-column' started to take
>    into account invisibility. (bug#56837)

This cannot be avoided.  Emacs provides by default many features that
"mess up" this kind of "true textual indentation".  Some of these
features include:

  . character composition
  . tab expansion
  . display properties that hide text and display some other text
  . display properties that align text
  . faces that affect character metrics via fonts
  . invisible text

(I'm probably missing some more.)  Only the Lisp program knows which
of those are relevant to the particular application at hand.  For
example, in your "true textual indentation", how do you account for
U+0061 LATIN SMALL LETTER A followed by U+0301 COMBINING ACUTE ACCENT?
They are almost always displayed as a single glyph: á.  Do you
consider this a single column or 2 columns?  What about long Emoji
sequences?

We could introduce separate indentation/current-column knobs for each
of the above features, but it would make little sense to me, since
most, if not all, of them already have such knobs.  For example,
character composition can be disabled by flipping a variable.

> > I don't insist in making that change.  Quite the opposite, actually.
> > I also expect it to break gobs of indentation code where invisible
> > text is involved.  Indentation code should probably temporarily remove
> > the invisibility spec, while indenting, or something.
> 
> That would make sense, yes.
> 
> > The main motivation to fix scan_for_column to consider more visual
> > effects was so vertical cursor motion works as expected when large
> > portions of text are hidden.
> 
> What about having something like `current-visual-column' that will be
> used when we really need to examine which display column the cursor is
> it, accounting for all the display-affecting properties?
> 
> Or maybe even have `use-visual-columns' variable that will modify how
> `current-column' behaves ('visual, 'textual, 'visual-ignore-invisible,
> etc)

See above: you are actually opening a Pandora box, and any solution
will likely be based on existing variables.  So I think we already
have what you want, it just might not be immediately obvious in each
case which of the knobs to use.  The default behavior of
current-column can already be affected by disabling
auto-composition-mode, by tweaking the buffer's invisibility spec, and
by defining display properties whose values are conditional.  Tab
expansion can also be controlled.  What else would make sense?

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> 1. Org mode is generally aiming for the produced Org files to be
>>    readable as unfontified plain text. So, quirks related to visual
>>    indentation generally tend to mess things up.
>
> "Unfontified"?  AFAIK, Org files do use fontifications, don't they?
> So what do you mean by "unfontified plain text" here?  But that's an
> aside, feel free to ignore.

For example, consider an Org table like

| *This* | is | some text |
| more |    |  text     |

It looks aligned in Org buffers ("*" is invisible), but not when copied
to message buffer.

>> 2. Org mode uses indentation as part of the syntax. I had to get rid of
>>    using `current-column' and calculated "true textual indentation"
>>    manually to avoid breakage after `current-column' started to take
>>    into account invisibility. (bug#56837)
>
> This cannot be avoided.  Emacs provides by default many features that
> "mess up" this kind of "true textual indentation".  Some of these
> features include:
> ...
> (I'm probably missing some more.)  Only the Lisp program knows which
> of those are relevant to the particular application at hand.

Sure, but how can an application, say, disable all the effects of visual
representation without (1) searching and let-binding each specific
toggle (and possibly adding more in future versions of Emacs); (2)
re-implementing the existing indentation code (like what
`org-current-text-column' does)?

> We could introduce separate indentation/current-column knobs for each
> of the above features, but it would make little sense to me, since
> most, if not all, of them already have such knobs.  For example,
> character composition can be disabled by flipping a variable.

It would help to list what contributes to indentation/columns in the
documentation.

> See above: you are actually opening a Pandora box, and any solution
> will likely be based on existing variables.  So I think we already
> have what you want, it just might not be immediately obvious in each
> case which of the knobs to use.  The default behavior of
> current-column can already be affected by disabling
> auto-composition-mode, by tweaking the buffer's invisibility spec, and
> by defining display properties whose values are conditional.  Tab
> expansion can also be controlled.  What else would make sense?

A toggle: disable all visual contributors.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Mon, 24 Jul 2023 08:18:54 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> 1. Org mode is generally aiming for the produced Org files to be
> >>    readable as unfontified plain text. So, quirks related to visual
> >>    indentation generally tend to mess things up.
> >
> > "Unfontified"?  AFAIK, Org files do use fontifications, don't they?
> > So what do you mean by "unfontified plain text" here?  But that's an
> > aside, feel free to ignore.
> 
> For example, consider an Org table like
> 
> | *This* | is | some text |
> | more |    |  text     |
> 
> It looks aligned in Org buffers ("*" is invisible), but not when copied
> to message buffer.

Where does fontification enter this picture?

> >> 2. Org mode uses indentation as part of the syntax. I had to get rid of
> >>    using `current-column' and calculated "true textual indentation"
> >>    manually to avoid breakage after `current-column' started to take
> >>    into account invisibility. (bug#56837)
> >
> > This cannot be avoided.  Emacs provides by default many features that
> > "mess up" this kind of "true textual indentation".  Some of these
> > features include:
> > ...
> > (I'm probably missing some more.)  Only the Lisp program knows which
> > of those are relevant to the particular application at hand.
> 
> Sure, but how can an application, say, disable all the effects of visual
> representation without (1) searching and let-binding each specific
> toggle (and possibly adding more in future versions of Emacs); (2)
> re-implementing the existing indentation code (like what
> `org-current-text-column' does)?

It can't.  But then it also doesn't want to.  In practice, most of
these features should not be turned off, because either the text will
become illegible, or the indentation performed when the features are
turned off will break when the text is displayed "normally".

IOW, the tendency should be to provide _more_ visual indentation, by
making our indentation commands smarter and more fine-grained (e.g.,
pixel-wise), not to make them _less_ visual by disabling the important
display features.

The important thing to remember is that Emacs makes all those
display-time transformation because that's how people want to see the
text on the screen.  It is very rare to see an application that wants
to show decomposed characters, as in a◌́ instead of á, or to see a TAB
shown as a single column.  Heck, even the display of control
characters, like \x01, is part of this, and why would we want to turn
that off?

IOW, the need for turning these off is extremely rare, and doesn't
justify such global toggles, because no one will use them.

> > We could introduce separate indentation/current-column knobs for each
> > of the above features, but it would make little sense to me, since
> > most, if not all, of them already have such knobs.  For example,
> > character composition can be disabled by flipping a variable.
> 
> It would help to list what contributes to indentation/columns in the
> documentation.

They are a legion.  Basically, every display-related feature described
in the ELisp manual -- and there are a lot of them -- is of this
nature.  Since we already describe them all in the manual, adding a
section which mentions them all together is strictly not necessary for
a reference manual.  It's more a job for a tutorial.

You are asking that someone does a very large job of collecting
existing stuff together, for facilitating a solution of some pretty
rare problem.  I cannot justify a large job such as this one -- going
through all the Emacs display features and describing them together --
for this kind of purpose.  But if someone wants to work on that, I
won't necessarily object if the result is concise and doesn't repeat
the existing material.

> > See above: you are actually opening a Pandora box, and any solution
> > will likely be based on existing variables.  So I think we already
> > have what you want, it just might not be immediately obvious in each
> > case which of the knobs to use.  The default behavior of
> > current-column can already be affected by disabling
> > auto-composition-mode, by tweaking the buffer's invisibility spec, and
> > by defining display properties whose values are conditional.  Tab
> > expansion can also be controlled.  What else would make sense?
> 
> A toggle: disable all visual contributors.

It will never be used.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> For example, consider an Org table like
>> 
>> | *This* | is | some text |
>> | more |    |  text     |
>> 
>> It looks aligned in Org buffers ("*" is invisible), but not when copied
>> to message buffer.
>
> Where does fontification enter this picture?

"*" is made invisible after fontification.

Fontified:

 | This | is | some text |
 | more |    |  text     |

Unfontified

 | *This* | is | some text |
 | more |    |  text     |

> IOW, the tendency should be to provide _more_ visual indentation, by
> making our indentation commands smarter and more fine-grained (e.g.,
> pixel-wise), not to make them _less_ visual by disabling the important
> display features.
>
> The important thing to remember is that Emacs makes all those
> display-time transformation because that's how people want to see the
> text on the screen.  It is very rare to see an application that wants
> to show decomposed characters, as in a◌́ instead of á, or to see a TAB
> shown as a single column.  Heck, even the display of control
> characters, like , is part of this, and why would we want to turn
> that off?
>
> IOW, the need for turning these off is extremely rare, and doesn't
> justify such global toggles, because no one will use them.

I can see your point. However, this is sometimes conflicting with
copying text verbatim or viewing it in other editors. For example,
nameless-mode that visually compresses
my-long-package-name-variable-name into :variable-name creates a lot of
mess when the same file is committed to public repo and later opened by
other contributors without nameless-mode enabled.

In the ideal world, Emacs would indent both visually and textually. With
visual part only using 'display text properties that do not modify the
actual text in file.

>> It would help to list what contributes to indentation/columns in the
>> documentation.
>
> They are a legion.  Basically, every display-related feature described
> in the ELisp manual -- and there are a lot of them -- is of this
> nature.  Since we already describe them all in the manual, adding a
> section which mentions them all together is strictly not necessary for
> a reference manual.  It's more a job for a tutorial.
>
> You are asking that someone does a very large job of collecting
> existing stuff together, for facilitating a solution of some pretty
> rare problem.  I cannot justify a large job such as this one -- going
> through all the Emacs display features and describing them together --
> for this kind of purpose.  But if someone wants to work on that, I
> won't necessarily object if the result is concise and doesn't repeat
> the existing material.

I was mostly advocating the need in "disable them all" toggle as a less
maintenance-heavy alternative.

>> A toggle: disable all visual contributors.
>
> It will never be used.

I would use it in Org instead of `org-current-text-column'.
It currently relies upon `string-width' ignoring visuals, which may or
may not hold in future (the docstring implies that `string-width' may as
well consider visuals: "Return width of STRING when displayed in the
current buffer.")

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Tue, 25 Jul 2023 08:38:38 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> For example, consider an Org table like
> >> 
> >> | *This* | is | some text |
> >> | more |    |  text     |
> >> 
> >> It looks aligned in Org buffers ("*" is invisible), but not when copied
> >> to message buffer.
> >
> > Where does fontification enter this picture?
> 
> "*" is made invisible after fontification.

So these are not font-lock-* faces, right?

> > The important thing to remember is that Emacs makes all those
> > display-time transformation because that's how people want to see the
> > text on the screen.  It is very rare to see an application that wants
> > to show decomposed characters, as in a◌́ instead of á, or to see a TAB
> > shown as a single column.  Heck, even the display of control
> > characters, like , is part of this, and why would we want to turn
> > that off?
> >
> > IOW, the need for turning these off is extremely rare, and doesn't
> > justify such global toggles, because no one will use them.
> 
> I can see your point. However, this is sometimes conflicting with
> copying text verbatim or viewing it in other editors. For example,
> nameless-mode that visually compresses
> my-long-package-name-variable-name into :variable-name creates a lot of
> mess when the same file is committed to public repo and later opened by
> other contributors without nameless-mode enabled.

This might mean we need more intelligent commands that copy text
elsewhere.  However, note that it is OK for Emacs to expect other
software/media to share at least some of the visual features.  For
example, I have yet to see a text-rendering program that doesn't show
diacritics composed with the base characters, and most show Emoji as
you'd expect, not as a series of control codes.  OTOH, tabs could be
shown differently, especially if tab-width was customized.

But I think this is a relatively far tangent, as it is not immediately
related to indentation in Emacs itself.  In Emacs, indentation is a
display-oriented feature, so the few programming languages where
indentation is syntactically and semantically meaningful must do
something special when the buffer uses non-trivial Emacs display
features beyond tab expansion.  Invisible text, in particular, doesn't
go along well with such use cases.

> In the ideal world, Emacs would indent both visually and textually.

I disagree that this is the ideal.  "Textual" indentation is not
indentation at all, it is fundamentally a completely different feature
for completely different purposes.  Even if we agree that Emacs should
have it (and I don't yet agree), you shouldn't expect from the Emacs
indentation to fill this niche, except by sheer luck in some use
cases.

> With visual part only using 'display text properties that do not
> modify the actual text in file.

This will (a) not help you, because the issue of width of the
whitespace stretches will still be pertinent; and (b) will complicate
Emacs much more, because copying such "text" will become much more
tricky in general.

But if you want to work on adding this, please feel free.  It has its
uses, even for indentation; see, for example, pixel-fill.el.  It is on
our TODO to provide pixel-granularity indentation for text using
variable-pitch fonts (but this again is for visual appearance only).

> >> A toggle: disable all visual contributors.
> >
> > It will never be used.
> 
> I would use it in Org instead of `org-current-text-column'.

So we have one user.  And even in your case, you don't want them all
disabled: the character compositions, for example, should stay turned
ON.

> may not hold in future (the docstring implies that `string-width' may as
> well consider visuals: "Return width of STRING when displayed in the
> current buffer.")

Selective citation alert!  The full text of the doc string is:

  Return width of STRING when displayed in the current buffer.
  Width is measured by how many columns it occupies on the screen.
  Optional arguments FROM and TO specify the substring of STRING to
  consider, and are interpreted as in ‘substring’.

  When calculating width of a multibyte character in STRING,
  only the base leading-code is considered; the validity of
  the following bytes is not checked.  Tabs in STRING are always
  taken to occupy ‘tab-width’ columns.  The effect of faces and fonts
  used for non-Latin and other unusual characters (such as emoji) is
  ignored as well, as are display properties and invisible text.
  For these reasons, the results are not generally reliable;
  for accurate dimensions of text as it will be displayed,
  use ‘window-text-pixel-size’ instead.

What else do you want us to say, for you to understand that the
"visual" part here is quite limited?

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> In the ideal world, Emacs would indent both visually and textually.
>
> I disagree that this is the ideal.  "Textual" indentation is not
> indentation at all, it is fundamentally a completely different feature
> for completely different purposes.  Even if we agree that Emacs should
> have it (and I don't yet agree), you shouldn't expect from the Emacs
> indentation to fill this niche, except by sheer luck in some use
> cases.

Got it.

>> With visual part only using 'display text properties that do not
>> modify the actual text in file.
>
> This will (a) not help you, because the issue of width of the
> whitespace stretches will still be pertinent; and (b) will complicate
> Emacs much more, because copying such "text" will become much more
> tricky in general.

I am not sure if I understand the problem with copying. Certain text
properties are already ignored when copying anyway.

> But if you want to work on adding this, please feel free.  It has its
> uses, even for indentation; see, for example, pixel-fill.el.  It is on
> our TODO to provide pixel-granularity indentation for text using
> variable-pitch fonts (but this again is for visual appearance only).

I did some preliminary work in Org mode.
For example, I tried to right-align tags like

|left fringe|* Heading                             :tag:|right fringe|

Using :align-to space spec and font-lock-keywords.
This can work, although it is unfortunate that there is no "stretch"
space that will automatically occupy as much space as possible without
pushing the line across right fringe.

Org also provides org-indent-mode that uses text properties to align
text visually.

>> I would use it in Org instead of `org-current-text-column'.
>
> So we have one user.  And even in your case, you don't want them all
> disabled: the character compositions, for example, should stay turned
> ON.

Sure. Especially if the composition is dictated by UTF standard.

>> may not hold in future (the docstring implies that `string-width' may as
>> well consider visuals: "Return width of STRING when displayed in the
>> current buffer.")
>
> Selective citation alert!  The full text of the doc string is:
>
>   Return width of STRING when displayed in the current buffer.
>   Width is measured by how many columns it occupies on the screen.
>   Optional arguments FROM and TO specify the substring of STRING to
>   consider, and are interpreted as in ‘substring’.
>
>   When calculating width of a multibyte character in STRING,
>   only the base leading-code is considered; the validity of
>   the following bytes is not checked.  Tabs in STRING are always
>   taken to occupy ‘tab-width’ columns.  The effect of faces and fonts
>   used for non-Latin and other unusual characters (such as emoji) is
>   ignored as well, as are display properties and invisible text.
>   For these reasons, the results are not generally reliable;
>   for accurate dimensions of text as it will be displayed,
>   use ‘window-text-pixel-size’ instead.
>
> What else do you want us to say, for you to understand that the
> "visual" part here is quite limited?

It is clear that it is limited, but I am concerned that there are still
some display features (and possibly display features added in future)
that may change the behavior.

I think that the main source of the confusion is the first line "Return
width of STRING when displayed in the current buffer", which sounds like
certain buffer-specific display things are affecting the result.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Thu, 27 Jul 2023 08:58:54 +0000
> 
> >> With visual part only using 'display text properties that do not
> >> modify the actual text in file.
> >
> > This will (a) not help you, because the issue of width of the
> > whitespace stretches will still be pertinent; and (b) will complicate
> > Emacs much more, because copying such "text" will become much more
> > tricky in general.
> 
> I am not sure if I understand the problem with copying. Certain text
> properties are already ignored when copying anyway.

Display properties are only supported in Emacs, so copying "outside"
will cause the text to look differently.  Likewise in Emacs, if
display properties are removed when copying.

> Using :align-to space spec and font-lock-keywords.
> This can work, although it is unfortunate that there is no "stretch"
> space that will automatically occupy as much space as possible without
> pushing the line across right fringe.

Of course, there is: use the 'right' position with a negative offset.

> I think that the main source of the confusion is the first line "Return
> width of STRING when displayed in the current buffer", which sounds like
> certain buffer-specific display things are affecting the result.

Feel free to suggest a single short enough sentence which doesn't have
this issue, and yet does explain that the value is not just the count
of characters in the string.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> For example, consider an Org table like
>
> | *This* | is | some text |
> | more |    |  text     |
>
> It looks aligned in Org buffers ("*" is invisible),

I just tried it in a vanilla Emacs-28 and "*" is not made invisible,
because `org-hide-emphasis-markers` defaults to nil.

> but not when copied to message buffer.

I think what you really want is that the above table be properly aligned
regardless of the value of `org-hide-emphasis-markers` and without
changing the buffer's char contents when changing the value of
`org-hide-emphasis-markers`, and I don't think you can do that without
adding `display` space properties dynamically in accordance with the
current value of `org-hide-emphasis-markers`.

Presumably you'll also want to keep columns aligned dynamically as text
is inserted/deleted.  I suspect that once you try to do that, it's
fairly easy to temporarily let-bind `buffer-invisibility-spec` around
the code that inserts SPCs&TABs to enforce the "vanilla" alignment.

Also enforcing this property that the text will be aligned both while
prettified in Emacs and when displayed in a text terminal may come with
the downside that some columns in Emacs will be "way too large" because
its width is determined by the width of the non-prettified text.

> Sure, but how can an application, say, disable all the effects of visual
> representation without (1) searching and let-binding each specific

Define "disable all the effects of visual representation".  I think what
you mean is something like "pretend the text is displayed by sending it
to an text terminal", but the width can depend on the specific text
terminal (especially for non-ASCII Unicode chars, but also for chars <
32).  So it's a non-trivial problem, in reality.


        Stefan

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Using :align-to space spec and font-lock-keywords.
>> This can work, although it is unfortunate that there is no "stretch"
>> space that will automatically occupy as much space as possible without
>> pushing the line across right fringe.
>
> Of course, there is: use the 'right' position with a negative offset.

This indeed works, but the annoying part is calculating the offset. One
needs to use `string-pixel-width', which is not always reliable.(Org has
its own `org-string-width', but it is still not good enough and
sometimes creates a few pixel overflows leading to line wraps).

The most robust way would be delegating offset calculation to the
redisplay of the actual line being displayed. That will guarantee
absence of inconsistencies. That's why I asked for "stretch" space
feature.

>> I think that the main source of the confusion is the first line "Return
>> width of STRING when displayed in the current buffer", which sounds like
>> certain buffer-specific display things are affecting the result.
>
> Feel free to suggest a single short enough sentence which doesn't have
> this issue, and yet does explain that the value is not just the count
> of characters in the string.

Return width of STRING when displayed using fixed width font.


However, if I look into the code, it looks like buffer-display-table is
also taking into account. And string_char_and_length, which I do not
fully understand. And glyphs appear to honor variable pitch font, if it
is default.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Presumably you'll also want to keep columns aligned dynamically as text
> is inserted/deleted.  I suspect that once you try to do that, it's
> fairly easy to temporarily let-bind `buffer-invisibility-spec` around
> the code that inserts SPCs&TABs to enforce the "vanilla" alignment.

Sure. Org is slowly moving in that direction - separating text alignment
and visual alignment. We have a persistent stream of related feature
requests.

However, remember that this thread started with `indent-to' calculating
target column without taking into account the display staff. While the
initial column is using `current-column' and considers
composition/faces/[partially] invisibility, the number of tabs/spaces to
be inserted to reach target column are calculated assuming that each
space occupies 1 column and each tab occupies tab-width columns.

Now, when we established that Emacs indentation is visual, it is clear
that the assumption in `indent-to' is not accurate.

>> Sure, but how can an application, say, disable all the effects of visual
>> representation without (1) searching and let-binding each specific
>
> Define "disable all the effects of visual representation".  I think what
> you mean is something like "pretend the text is displayed by sending it
> to an text terminal", but the width can depend on the specific text
> terminal (especially for non-ASCII Unicode chars, but also for chars <
> 32).  So it's a non-trivial problem, in reality.

You are right. Probably, a good goal would be "looks aligned in
fundamental-mode".

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Fri, 28 Jul 2023 08:06:54 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Using :align-to space spec and font-lock-keywords.
> >> This can work, although it is unfortunate that there is no "stretch"
> >> space that will automatically occupy as much space as possible without
> >> pushing the line across right fringe.
> >
> > Of course, there is: use the 'right' position with a negative offset.
> 
> This indeed works, but the annoying part is calculating the offset.

What offset?  You said "occupy as much space as possible", which means
the offset from the right edge is zero, right?

> The most robust way would be delegating offset calculation to the
> redisplay of the actual line being displayed.

That would require that the display engine scans the screen line
twice.  That's unacceptable, both for performance reasons and because
it violates the basic design of how the Emacs display iteration works.
Sorry, that won't fly.

Why does Org need to take up all the available space of a window to
begin with, btw?

> One needs to use `string-pixel-width', which is not always reliable.

string-pixel-width uses the display code, so if it is unreliable, so
will be any other implementation in the display engine (barring any
bugs).  (Of course, you didn't reveal any details of this lack of
reliability, so I don't really know what are we talking about here.)

> >> I think that the main source of the confusion is the first line "Return
> >> width of STRING when displayed in the current buffer", which sounds like
> >> certain buffer-specific display things are affecting the result.
> >
> > Feel free to suggest a single short enough sentence which doesn't have
> > this issue, and yet does explain that the value is not just the count
> > of characters in the string.
> 
> Return width of STRING when displayed using fixed width font.

That loses information (the "current buffer" part: it's important).

Can you explain why "fixed width font" is important enough for you to
want to see it there?  After all, the function counts columns, so
whether the font is fixed-pitch or variable-pitch shouldn't matter.

> However, if I look into the code, it looks like buffer-display-table is
> also taking into account.

Yes.  Also character compositions.

> And string_char_and_length, which I do not fully understand.

That's unrelated.  string_char_and_length is just a convenient way of
walking multibyte text one character at a time.  It returns the
codepoint of the current character, and also returns the length of its
multibyte sequence, so it is easy to get to the next character in one
jump.  (Keep in mind that the basic movement across buffer text is
always by bytes, and a character can be represented by 1 to 5 bytes.)

> And glyphs appear to honor variable pitch font, if it is default.

No, they don't.  When the function finds characters that will be
composed on display, it computes the pixel-width of the result of the
composition, and then converts that into the units of the frame's
default face's font.  For that conversion, and for that conversion
only, the function needs the parameter of the default face's font that
tells us the width of its characters; if that font is variable-pitch
(unlikely), then these parameters give only some kind of average
width.

But again, all this access to the font parameters is only done for
composed characters, because character composition rules can produce
glyphs that have no corresponding codepoints, and therefore we cannot
look them up in char-width-table.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 64696@debbugs.gnu.org
> Date: Fri, 28 Jul 2023 08:30:47 +0000
> 
> However, remember that this thread started with `indent-to' calculating
> target column without taking into account the display staff. While the
> initial column is using `current-column' and considers
> composition/faces/[partially] invisibility, the number of tabs/spaces to
> be inserted to reach target column are calculated assuming that each
> space occupies 1 column and each tab occupies tab-width columns.
> 
> Now, when we established that Emacs indentation is visual, it is clear
> that the assumption in `indent-to' is not accurate.

Since indent-to inserts only tabs and spaces, how is that assumption
inaccurate?

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Now, when we established that Emacs indentation is visual, it is clear
>> that the assumption in `indent-to' is not accurate.
>
> Since indent-to inserts only tabs and spaces, how is that assumption
> inaccurate?

Width of spaces in particular can vary depending on the text properties.
(:width and :relative-width in 41.16.2 Specified Spaces section of Elisp manual)
Width of tabs can be controlled by tabs stops (AFAIU, I am not familiar
with this feature).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Fri, 28 Jul 2023 12:26:55 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Now, when we established that Emacs indentation is visual, it is clear
> >> that the assumption in `indent-to' is not accurate.
> >
> > Since indent-to inserts only tabs and spaces, how is that assumption
> > inaccurate?
> 
> Width of spaces in particular can vary depending on the text properties.
> (:width and :relative-width in 41.16.2 Specified Spaces section of Elisp manual)
> Width of tabs can be controlled by tabs stops (AFAIU, I am not familiar
> with this feature).

So you are saying that indent-to should take care of not inheriting
text properties from preceding text?  Or are there any other problems?

As for tabs, the width is controlled by tab-width, but indent-to
already takes that into consideration.  Am I missing something?  (If
you are thinking about tab-stop-list, that is only used by commands
that move or indent to tab stops, AFAIK.)

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Width of spaces in particular can vary depending on the text properties.
>> (:width and :relative-width in 41.16.2 Specified Spaces section of Elisp manual)
>> Width of tabs can be controlled by tabs stops (AFAIU, I am not familiar
>> with this feature).
>
> So you are saying that indent-to should take care of not inheriting
> text properties from preceding text?  Or are there any other problems?

AFAIU, we concluded previously that non-inheriting text properties is a
no-go because it will likely break things.
So, I think that `indent-to' should take care calculating space width
accurately in the presence of :width and similar specs.

Not sure what to do with :align-to.

> As for tabs, the width is controlled by tab-width, but indent-to
> already takes that into consideration.  Am I missing something?  (If
> you are thinking about tab-stop-list, that is only used by commands
> that move or indent to tab stops, AFAIK.)

I was thinking about tab-stop-list, but, as I said, I am not familiar
with it. If you say that tab-width is the only single thing influencing
the display, then I do not see any problem.

... well, except the generic problem that `indent-to' may inherit
'display or 'invisible property that completely modifies how the
inserted spaces/tabs are displayed. But I do not see any useful way to
account for 'display/'invisible other than by ignoring it. Changing
number of spaces/tabs will not affect the resulting display in such
scenarios.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Fri, 28 Jul 2023 13:02:14 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So you are saying that indent-to should take care of not inheriting
> > text properties from preceding text?  Or are there any other problems?
> 
> AFAIU, we concluded previously that non-inheriting text properties is a
> no-go because it will likely break things.

I'm not talking about all the properties, I'm talking only about
those that affect the visible width of whitespace.  Basically, only
'display' properties, I think.

> So, I think that `indent-to' should take care calculating space width
> accurately in the presence of :width and similar specs.
> 
> Not sure what to do with :align-to.

These could be accounted for, but some 'display' properties cannot.
For example, if the property value is a string, we will be unable to
indent at all.

> ... well, except the generic problem that `indent-to' may inherit
> 'display or 'invisible property that completely modifies how the
> inserted spaces/tabs are displayed.

Which is why 'invisible' should not be inherited by indent-to, if we
want it to do a better job.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Of course, there is: use the 'right' position with a negative offset.
>> 
>> This indeed works, but the annoying part is calculating the offset.
>
> What offset?  You said "occupy as much space as possible", which means
> the offset from the right edge is zero, right?

I meant:

^some text<stretch space>some more text$

With <stretch space> adjusting its width in such a way that the whole
line occupies exactly buffer width.

Now, I have to use

^some text<:align-to `(- right-fringe ,(string-pixel-width "some more text"))>some more text$

>> The most robust way would be delegating offset calculation to the
>> redisplay of the actual line being displayed.
>
> That would require that the display engine scans the screen line
> twice.  That's unacceptable, both for performance reasons and because
> it violates the basic design of how the Emacs display iteration works.
> Sorry, that won't fly.

I do not think that double scan will be needed. Just space width
calculation is to be postponed until the line is processed.

May you point me to the place in code where the line scan is performed?

> Why does Org need to take up all the available space of a window to
> begin with, btw?

To produce right-aligned text columns:

* Heading                                              :tag1:tag2:tag3:
* Another heading                                                :tag4:

>> One needs to use `string-pixel-width', which is not always reliable.
>
> string-pixel-width uses the display code, so if it is unreliable, so
> will be any other implementation in the display engine (barring any
> bugs).  (Of course, you didn't reveal any details of this lack of
> reliability, so I don't really know what are we talking about here.)

Because `string-pixel-width' does not account for display and
fontification settings in current-buffer. For example, buffer-local
invisibility specs will be ignored; display tables will be ignored; face
remapping will be ignored.

There is also a number of edge cases when `string-pixel-width' will
straight return wrong width. For example, when string has
line-prefix/wrap-prefix text property, its width will depend on whether
string is displayed at bol or not. `string-pixel-width' will always make
it as if string is displayed at bol, thus adding line-prefix width to
the string.

>> Return width of STRING when displayed using fixed width font.
>
> That loses information (the "current buffer" part: it's important).

> Can you explain why "fixed width font" is important enough for you to
> want to see it there?  After all, the function counts columns, so
> whether the font is fixed-pitch or variable-pitch shouldn't matter.

My original concern was about `string-width' not producing reliable
results when Emacs visual settings are changed. And, indeed, some
visuals that are not detailed in the docstring are taken into account.

The purpose of using `string-width' in `org-current-text-column' is to
produce reliable result for different users with different visual
settings and fonts. This is because indentation is used in Org syntax
and must not be broken if the Org file is unchanged and visual settings
are.

>> And glyphs appear to honor variable pitch font, if it is default.
>
> No, they don't.  When the function finds characters that will be
> composed on display, it computes the pixel-width of the result of the
> composition, and then converts that into the units of the frame's
> default face's font.  For that conversion, and for that conversion
> only, the function needs the parameter of the default face's font that
> tells us the width of its characters; if that font is variable-pitch
> (unlikely), then these parameters give only some kind of average
> width.

I am concerned exactly about this unlikely scenario.
I have seen variable pitch creating one-off errors for width in the
past.

> But again, all this access to the font parameters is only done for
> composed characters, because character composition rules can produce
> glyphs that have no corresponding codepoints, and therefore we cannot
> look them up in char-width-table.

So, it is something Org needs to ignore somehow.
Org syntax must not depend on Emacs visual settings.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> AFAIU, we concluded previously that non-inheriting text properties is a
>> no-go because it will likely break things.
>
> I'm not talking about all the properties, I'm talking only about
> those that affect the visible width of whitespace.  Basically, only
> 'display' properties, I think.

Sounds reasonable.
Do you mean that `indent-to' will only inherit certain 'display
properties and use them to calculate the number of spaces to be
inserted?

>> Not sure what to do with :align-to.
>
> These could be accounted for, but some 'display' properties cannot.
> For example, if the property value is a string, we will be unable to
> indent at all.

If there is an 'invisible overlay on top (or overlay with 'display
property set to string), visual indentation will also be impossible no
matter how many spaces are inserted.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sat, 29 Jul 2023 09:00:22 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The most robust way would be delegating offset calculation to the
> >> redisplay of the actual line being displayed.
> >
> > That would require that the display engine scans the screen line
> > twice.  That's unacceptable, both for performance reasons and because
> > it violates the basic design of how the Emacs display iteration works.
> > Sorry, that won't fly.
> 
> I do not think that double scan will be needed. Just space width
> calculation is to be postponed until the line is processed.

That's what I meant by "double scan".  The Emacs display engine never
looks back, once it processed some text property.  Looking back is
particularly unacceptable when the display engine is called to start
in the middle of the line, because it would mean we need always go to
the beginning of the line.

> May you point me to the place in code where the line scan is performed?

There are two places: display_line and move_it_in_display_line_to.

> > Why does Org need to take up all the available space of a window to
> > begin with, btw?
> 
> To produce right-aligned text columns:
> 
> * Heading                                              :tag1:tag2:tag3:
> * Another heading                                                :tag4:

I guessed that much, but my point is: why not set some reasonable
fixed right position, and align to that?

> >> One needs to use `string-pixel-width', which is not always reliable.
> >
> > string-pixel-width uses the display code, so if it is unreliable, so
> > will be any other implementation in the display engine (barring any
> > bugs).  (Of course, you didn't reveal any details of this lack of
> > reliability, so I don't really know what are we talking about here.)
> 
> Because `string-pixel-width' does not account for display and
> fontification settings in current-buffer.  For example, buffer-local
> invisibility specs will be ignored; display tables will be ignored; face
> remapping will be ignored.

If that is the problem, you should use buffer-text-pixel-size or
window-text-pixel-size instead.

> There is also a number of edge cases when `string-pixel-width' will
> straight return wrong width. For example, when string has
> line-prefix/wrap-prefix text property, its width will depend on whether
> string is displayed at bol or not. `string-pixel-width' will always make
> it as if string is displayed at bol, thus adding line-prefix width to
> the string.

Does window-text-pixel-size work better in those cases?

> >> Return width of STRING when displayed using fixed width font.
> >
> > That loses information (the "current buffer" part: it's important).
> 
> > Can you explain why "fixed width font" is important enough for you to
> > want to see it there?  After all, the function counts columns, so
> > whether the font is fixed-pitch or variable-pitch shouldn't matter.
> 
> My original concern was about `string-width' not producing reliable
> results when Emacs visual settings are changed. And, indeed, some
> visuals that are not detailed in the docstring are taken into account.
> 
> The purpose of using `string-width' in `org-current-text-column' is to
> produce reliable result for different users with different visual
> settings and fonts. This is because indentation is used in Org syntax
> and must not be broken if the Org file is unchanged and visual settings
> are.

How does your proposal for the change in the doc string serve these
issues?  I asked a specific question about your proposed change, and I
don't think what you say above answers that specific question?

> >> And glyphs appear to honor variable pitch font, if it is default.
> >
> > No, they don't.  When the function finds characters that will be
> > composed on display, it computes the pixel-width of the result of the
> > composition, and then converts that into the units of the frame's
> > default face's font.  For that conversion, and for that conversion
> > only, the function needs the parameter of the default face's font that
> > tells us the width of its characters; if that font is variable-pitch
> > (unlikely), then these parameters give only some kind of average
> > width.
> 
> I am concerned exactly about this unlikely scenario.
> I have seen variable pitch creating one-off errors for width in the
> past.

If variable-pitch fonts are used for the _default_ face's font, then
using string-width as the measure of width on display is clearly
inadequate to begin with, isn't it?  The function tries to do its
best, but why do you expect it to do miracles?  Using variable-pitch
fonts for the default face in Emacs is asking for trouble, even if Org
is not used at all.  We currently don't support that well.

> > But again, all this access to the font parameters is only done for
> > composed characters, because character composition rules can produce
> > glyphs that have no corresponding codepoints, and therefore we cannot
> > look them up in char-width-table.
> 
> So, it is something Org needs to ignore somehow.

I don't understand why "ignore".  But TBH, I don't think I understand
what are you trying to accomplish with this long discussion.

> Org syntax must not depend on Emacs visual settings.

If that is what you are trying to do, I think you will have quite a
few difficulties.  Emacs does not cater to such applications,
especially when you are using APIs whose purpose was to support
display and layout calculations.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> I do not think that double scan will be needed. Just space width
>> calculation is to be postponed until the line is processed.
>
> That's what I meant by "double scan".  The Emacs display engine never
> looks back, once it processed some text property.  Looking back is
> particularly unacceptable when the display engine is called to start
> in the middle of the line, because it would mean we need always go to
> the beginning of the line.

What about fill_up_glyph_row_area_with_spaces and
fill_up_frame_row_with_spaces? They append extra space glyphs to build
up to window/frame width. And what I propose would enlarge some
existing space glyphs instead.

>> > Why does Org need to take up all the available space of a window to
>> > begin with, btw?
>> 
>> To produce right-aligned text columns:
>> 
>> * Heading                                              :tag1:tag2:tag3:
>> * Another heading                                                :tag4:
>
> I guessed that much, but my point is: why not set some reasonable
> fixed right position, and align to that?

That's what we do (org-tags-column). However, people often ask for
auto-adjustment to right margin when frame/window is resized.

The usual use-case for auto-adjustment is when Org/agenda window is
first built as a sole window in the frame and then user splits the frame
into two windows vertically:

* Heading                                              :tag1:tag2:tag3:
* Another heading                                                :tag4:

* Heading                           →| Other text file here.
* Another heading                   →| Tags on the left are not visible.

or

* Heading                           ⤶| Other text file here.
           :tag1:tag2:tag3:          | Tags wrapped and look ugly.
* Another heading                   ⤶|
                     :tag4:          |

> If that is the problem, you should use buffer-text-pixel-size or
> window-text-pixel-size instead.

Good point. Thanks!

>> My original concern was about `string-width' not producing reliable
>> results when Emacs visual settings are changed. And, indeed, some
>> visuals that are not detailed in the docstring are taken into account.
>> 
>> The purpose of using `string-width' in `org-current-text-column' is to
>> produce reliable result for different users with different visual
>> settings and fonts. This is because indentation is used in Org syntax
>> and must not be broken if the Org file is unchanged and visual settings
>> are.
>
> How does your proposal for the change in the doc string serve these
> issues?  I asked a specific question about your proposed change, and I
> don't think what you say above answers that specific question?

I was referring to one step earlier in the discussion:

    >> A toggle: disable all visual contributors.
    > It will never be used.

    I would use it in Org instead of `org-current-text-column'.
    It currently relies upon `string-width' ignoring visuals, which may or
    may not hold in future (the docstring implies that `string-width' may as
    well consider visuals: "Return width of STRING when displayed in the
    current buffer.")

You pointed that my docstring reference did not include the whole
docstring and that the docstring, in fact, does say that visuals have
"limited" effect. So, I proceeded with docstring suggestion to highlight
that window settings have no effect.

My suggestion turned out to be wrong: (1) variable pitch faces can
affect the results for glyphs; (2) buffer-display-table can alter the
results.

And note that the full docstring of `string-width' does not mention the
above 2 factors. It just mentions a small subset of visual settings that
_do not_ affect the results. Which is deceiving.

> If variable-pitch fonts are used for the _default_ face's font, then
> using string-width as the measure of width on display is clearly
> inadequate to begin with, isn't it?

So, we coming back to the original discussion about a toggle to disable
"visuals", there seems to be a need in it, at the end.
`org-current-text-column' is clearly not 100% reliable when using
`string-width'.

> I don't understand why "ignore".  But TBH, I don't think I understand
> what are you trying to accomplish with this long discussion.

We have discussed multiple topics in this thread. From my point point of
view, things are mostly revolving around understanding how
invisible/display/other visual settings are affecting `indent-to', and
`current-column'. Because on Org mode side, we sometimes want and
sometimes don't want to account for visuals.

On Emacs side, I can see that full, partial, or limited subset of visual
settings is accounted for in various functions, like `indent-to',
`current-column', and `string-width'. Not everything is clearly
documented for `string-width' and accounting for invisible text
properties is not fully consistent in `indent-to'.

>> Org syntax must not depend on Emacs visual settings.
>
> If that is what you are trying to do, I think you will have quite a
> few difficulties.  Emacs does not cater to such applications,
> especially when you are using APIs whose purpose was to support
> display and layout calculations.

I'd prefer to use the existing API if possible. But to do it, I first
want to understand its logic. Inconsistencies in this logic is what this
report is about.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sun, 30 Jul 2023 07:51:09 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I do not think that double scan will be needed. Just space width
> >> calculation is to be postponed until the line is processed.
> >
> > That's what I meant by "double scan".  The Emacs display engine never
> > looks back, once it processed some text property.  Looking back is
> > particularly unacceptable when the display engine is called to start
> > in the middle of the line, because it would mean we need always go to
> > the beginning of the line.
> 
> What about fill_up_glyph_row_area_with_spaces and
> fill_up_frame_row_with_spaces? They append extra space glyphs to build
> up to window/frame width.

First, these are for TTY frames only (that's the meaning of
"frame-based redisplay" in Emacs display parlance).

And more importantly, these fill _the_rest_ of a screen line with
space glyphs.  We already know how to do that, on both TTY and GUI
frames, see extend_face_to_end_of_line, where it calls
append_stretch_glyph.  The problem is that when these functions are
called, we need to be able to calculate the width of the stretch glyph
to produce.  That is possible if the stretch is the last element of a
screen line and ends at the right edge of the window's text area, but
not possible if some text is yet to follow, because the width of the
following text on display is not yet known -- it was not calculated.

The basic design of the Emacs display engine is that layout
calculations proceed in strict left-to-right visual order, one buffer
or string position at a time, and each position is completely
processed before we proceed to the next.  What you are asking for will
violate this basic design, so it is something I very much would like
to avoid, because it will definitely cause complications and bugs.

> And what I propose would enlarge some existing space glyphs instead.

What do you mean by "enlarge existing space glyphs"?  In the functions
to which you pointed, each space glyph is just the SPC character
(which is eventually sent to the text-mode terminal); those cannot be
enlarged, because this is a text-mode terminal.

> >> > Why does Org need to take up all the available space of a window to
> >> > begin with, btw?
> >> 
> >> To produce right-aligned text columns:
> >> 
> >> * Heading                                              :tag1:tag2:tag3:
> >> * Another heading                                                :tag4:
> >
> > I guessed that much, but my point is: why not set some reasonable
> > fixed right position, and align to that?
> 
> That's what we do (org-tags-column). However, people often ask for
> auto-adjustment to right margin when frame/window is resized.

And you must honor each and every request?  Why?  There's nothing
wrong with telling users "this is too complicated to implement,
sorry".

> The usual use-case for auto-adjustment is when Org/agenda window is
> first built as a sole window in the frame and then user splits the frame
> into two windows vertically:

When a window is split, the alignment could be re-done, no?

> And note that the full docstring of `string-width' does not mention the
> above 2 factors. It just mentions a small subset of visual settings that
> _do not_ affect the results. Which is deceiving.

"Deceiving" is a harsh word, please reconsider.

There's a limit to which a doc string can describe every single
subtlety of a non-trivial implementation, and still remain useful.  If
someone needs to know about these aspects (presumably because their
Lisp program does something very special, because otherwise these
things "just work"), they should either read the source or ask here,
because they basically use string-width outside of its main usage
domain.

> > If variable-pitch fonts are used for the _default_ face's font, then
> > using string-width as the measure of width on display is clearly
> > inadequate to begin with, isn't it?
> 
> So, we coming back to the original discussion about a toggle to disable
> "visuals", there seems to be a need in it, at the end.

I don't see how this follows.  And I thought I already explained why
"visuals" is not well defined in general: each application needs to
disable some parts of it, but never all of them, and which parts
depends on the application.  Why do we have to reiterate this over and
over again?

> `org-current-text-column' is clearly not 100% reliable when using
> `string-width'.

Then don't use string-width.  I don't know what you should use
instead, because I don't understand well enough what
org-current-text-column wants to do and for what purpose.  It could be
that the functionality you want doesn't exist, or maybe it can be had
by some tweaking of the existing APIs, I don't know.  But if that is
the issue here, we should restart the discussion from a very different
point: by your describing what you need to do win
org-current-text-column and why.  What current-column and indent-to do
or don't do is almost irrelevant, it seems.

> I'd prefer to use the existing API if possible. But to do it, I first
> want to understand its logic. Inconsistencies in this logic is what this
> report is about.

There are no inconsistencies, not from where I stand.  Each API was
written for a specific purpose and does a specific job; if your
purpose is very different, you need to describe it first, and do that
with all the details.  Then we can see if we already support your
purpose or need new APIs.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> And note that the full docstring of `string-width' does not mention the
>> above 2 factors. It just mentions a small subset of visual settings that
>> _do not_ affect the results. Which is deceiving.
>
> "Deceiving" is a harsh word, please reconsider.

I did not mean being harsh. Maybe confusing is a better word.

> There's a limit to which a doc string can describe every single
> subtlety of a non-trivial implementation, and still remain useful.  If
> someone needs to know about these aspects (presumably because their
> Lisp program does something very special, because otherwise these
> things "just work"), they should either read the source or ask here,
> because they basically use string-width outside of its main usage
> domain.

I personally feel confused when looking at the available width
calculation in Emacs: `string-width', `string-pixel-width',
`window-text-pixel-size', `length' are all different, and sometimes in
subtle (and undocumented) ways. For me, the best way to resolve the
confusion would be more detailed docstrings that explain the
limitations. I have no better ideas.

Also, may you explain some parts of the `string-width' docstring?

1. "When calculating width of a multibyte character in STRING, only the
   base leading-code is considered; the validity of the following bytes
   is not checked."

   What does "validity" refer to and how is it related to width?

2. "Tabs in STRING are always taken to occupy tab-width columns."

   "always" in the above is not accurate when buffer-display-table
   replaces <TAB> display. Is it considered subtlety?

3. "The effect of faces and fonts used for non-Latin and other unusual
   characters (such as emoji) is ignored as well"

   Is the effect of faces not ignored for Latin characters? Or the faces
   and fonts are ignored completely?

4. "... , as are display properties and invisible text"

   What about other properties? Like 'line-prefix. Maybe "all the text

   properties are ignored"?

5. "For these reasons, the results are not generally reliable;"

   This sounds like "this function is not useful". May it be better to
   rewrite this as

     To calculate accurate text dimensions as it is displayed in current
     buffer, use `window-text-pixel-size'.

     To take into account faces and other text properties in STRING, use
     `string-pixel-width'.

> There are no inconsistencies, not from where I stand.  Each API was
> written for a specific purpose and does a specific job; if your
> purpose is very different, you need to describe it first, and do that
> with all the details.  Then we can see if we already support your
> purpose or need new APIs.

This is getting too far out of scope of the initial discussion. Let's
focus back on `indent-to'/`current-column' and related Emacs issues.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Sun, 30 Jul 2023 11:45:20 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > There's a limit to which a doc string can describe every single
> > subtlety of a non-trivial implementation, and still remain useful.  If
> > someone needs to know about these aspects (presumably because their
> > Lisp program does something very special, because otherwise these
> > things "just work"), they should either read the source or ask here,
> > because they basically use string-width outside of its main usage
> > domain.
> 
> I personally feel confused when looking at the available width
> calculation in Emacs: `string-width', `string-pixel-width',
> `window-text-pixel-size', `length' are all different, and sometimes in
> subtle (and undocumented) ways. For me, the best way to resolve the
> confusion would be more detailed docstrings that explain the
> limitations. I have no better ideas.

string-pixel-width and window-text-pixel-size are different varieties
of the same basic primitive.  Other than that, each of the functions
you mentioned serve a different purpose; in particular, 'length' is
entirely unrelated to the display width of a string.  When each API is
used for its purpose, there's no confusion and no subtleties, because
each one of them does its job, and the subtleties should not matter.
It's only when you want them to do some job they were not directly
designed for that the subtleties become important.  But your
insistence on documenting all of those technicalities is not TRT: it
will just make the documentation much harder to understand and use for
those who do use these APIs for their advertised purposes.

> 1. "When calculating width of a multibyte character in STRING, only the
>    base leading-code is considered; the validity of the following bytes
>    is not checked."

Remnant from a very distant past; ignore it.

> 2. "Tabs in STRING are always taken to occupy tab-width columns."
> 
>    "always" in the above is not accurate when buffer-display-table
>    replaces <TAB> display. Is it considered subtlety?

No, "always" disregarding the column where the TAB will be displayed.
A TAB that begins on column zero will be 8 columns wide; a TAB that
begins on column 5 will be only 3 columns wide.

> 3. "The effect of faces and fonts used for non-Latin and other unusual
>    characters (such as emoji) is ignored as well"
> 
>    Is the effect of faces not ignored for Latin characters?

The default face's font covers those, and if it's a fixed-pitch font,
the character width stored in char-width-table is adequate.

> 4. "... , as are display properties and invisible text"
> 
>    What about other properties? Like 'line-prefix. Maybe "all the text
>    properties are ignored"?

line-prefix doesn't affect the width of the string itself.

"All the text properties" is incorrect: at least 'composition' and
'invisible' aren't ignored.

> 5. "For these reasons, the results are not generally reliable;"
> 
>    This sounds like "this function is not useful".

I've now reworded that part.

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> 1. "When calculating width of a multibyte character in STRING, only the
>>    base leading-code is considered; the validity of the following bytes
>>    is not checked."
>
> Remnant from a very distant past; ignore it.

I see that you updated the docstring in recent commits. Thanks!

(Side note: Is there a way to track commits related to a bug report? I
saw this change by sheer luck.)

>> 4. "... , as are display properties and invisible text"
>> 
>>    What about other properties? Like 'line-prefix. Maybe "all the text
>>    properties are ignored"?
>
> line-prefix doesn't affect the width of the string itself.

Then, there is a bug in `string-pixel-width':

string-pixel-width is an autoloaded and natively compiled function
defined in subr-x.el.

Documentation
Return the width of STRING in pixels.

(string-pixel-width "asd") ;=> 30
(string-pixel-width (propertize "asd" 'line-prefix "123")) ;=> 60

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64696: 30.0.50; indent-to inherits preceding text properties, including 'invisible

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: monnier@iro.umontreal.ca, 64696@debbugs.gnu.org
> Date: Mon, 31 Jul 2023 07:18:16 +0000
> 
> (Side note: Is there a way to track commits related to a bug report? I
> saw this change by sheer luck.)

Every change that is related to some bug report, be it a change in
documentation or in the code, should mention the bug number in the
commit log message (if the committer doesn't forget).  In this case,
the change was so far away of the issue of the bug that I decided not
to mention the bug.

However, I did say:

> I've now reworded that part.

which should have hinted on the fact that some change of the doc
string was installed.

> >> 4. "... , as are display properties and invisible text"
> >> 
> >>    What about other properties? Like 'line-prefix. Maybe "all the text
> >>    properties are ignored"?
> >
> > line-prefix doesn't affect the width of the string itself.
> 
> Then, there is a bug in `string-pixel-width':

Possibly.  Please submit a separate bug report, and someone will look
into it.