bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Ihor Radchenko]

Hi,

Try to do the following:
1. emacs -Q
2. M-x outline-mode
3. Insert

* something
  a
* else

4. Move point to "* <point>something"
5. <TAB> to fold the outline
6. M-: (save-excursion (search-forward " a") (current-indentation))
   ;; => 2
7. M-: (remove-from-invisibility-spec '(outline . t))
8. M-: (add-to-invisibility-spec '(outline))
9. M-: (save-excursion (search-forward " a") (current-indentation))
   ;; => 0; expected: => 2

First reported in https://orgmode.org/list/87k0h49s7z.fsf@localhost   

Best,
Ihor

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Date: Sat, 30 Jul 2022 15:56:39 +0800
> 
> 1. emacs -Q
> 2. M-x outline-mode
> 3. Insert
> 
> * something
>   a
> * else
> 
> 4. Move point to "* <point>something"
> 5. <TAB> to fold the outline
> 6. M-: (save-excursion (search-forward " a") (current-indentation))
>    ;; => 2
> 7. M-: (remove-from-invisibility-spec '(outline . t))
> 8. M-: (add-to-invisibility-spec '(outline))
> 9. M-: (save-excursion (search-forward " a") (current-indentation))
>    ;; => 0; expected: => 2

I don't understand what is the purpose of asking about indentation of
a line that is completely invisible.  current-indentation skips
invisible characters, so if everything is invisible, why do you expect
any useful result?

Technically, the difference between the two results is because
current-indentation tries to account for the ellipsis (which is
present in the first case but not in the second).  But that's not
relevant to the larger issue at hand here.

Bottom line: I think you are hitting undefined behavior here.

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't understand what is the purpose of asking about indentation of
> a line that is completely invisible.  current-indentation skips
> invisible characters, so if everything is invisible, why do you expect
> any useful result?

This is needed for parsing. In Org, indentation defines the list level.
Similarly, indentation is meaningful in Python.

Since parsed text can be hidden inside fold (using org-mode,
or outline-mode; the latter may also be used in Python buffers), getting
an accurate indentation is important even when text is invisible.

> Technically, the difference between the two results is because
> current-indentation tries to account for the ellipsis (which is
> present in the first case but not in the second).  But that's not
> relevant to the larger issue at hand here.

I do not understand how ellipsis can be meaningfully accounted for with
regard to indentation. AFAIU, they are not. Try

* something
          a
* else

4. Move point to "* <point>something"
5. <TAB> to fold the outline
6. M-: (save-excursion (search-forward " a") (current-indentation))
   ;; => 10

> Bottom line: I think you are hitting undefined behavior here.

Maybe. But then I'd like to be able to get the "hard" indentation
information for the underlying text in buffer, accounting for tab-width,
and disregarding overlays and other font-lock staff.

As I mentioned above, this is required for parsing.

Best,
Ihor

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: 56837@debbugs.gnu.org
> Date: Sat, 30 Jul 2022 17:08:12 +0800
> 
> > Bottom line: I think you are hitting undefined behavior here.
> 
> Maybe. But then I'd like to be able to get the "hard" indentation
> information for the underlying text in buffer, accounting for tab-width,
> and disregarding overlays and other font-lock staff.

I guess you should temporarily remove the invisible property?  Like
this:

  (let ((invis-spec buffer-invisibility-spec)
        indent)
     (remove-from-invisibility-spec '(outline . t))
     (setq indent
           (save-excursion (search-forward " a") (current-indentation)))
     (setq buffer-invisibility-spec invis-spec) indent)

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> I guess you should temporarily remove the invisible property?  Like
> this:
>
>   (let ((invis-spec buffer-invisibility-spec)
>         indent)
>      (remove-from-invisibility-spec '(outline . t))
>      (setq indent
>            (save-excursion (search-forward " a") (current-indentation)))
>      (setq buffer-invisibility-spec invis-spec) indent)

A simpler version would be let-binding buffer-invisibility-spec.

Note that there is similar issue with current-column, though
current-column is much less likely to be used without considering
display properties.

Would it be possible to include the "text" versions of
current-indentation and current-column into Emacs?

Something like

(defmacro org-current-text-indentation ()
  "Like `current-indentation', but ignore display/invisible properties."
  `(let ((buffer-invisibility-spec nil))
     (current-indentation)))

(defmacro org-current-text-column ()
  "Like `current-column', but ignore display/invisible properties."
  `(string-width (buffer-substring-no-properties
		  (line-beginning-position) (point))))

If not, could you at least mention the invisible text issues in
`current-indentation' docstring? The current undefined behaviour is very
hard to debug.

Best,
Ihor

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: 56837@debbugs.gnu.org
> Date: Sat, 30 Jul 2022 19:27:09 +0800
> 
> A simpler version would be let-binding buffer-invisibility-spec.

Really?

> Note that there is similar issue with current-column, though
> current-column is much less likely to be used without considering
> display properties.

These are all display-related, in that their results are supposed to
closely reflect what will be shown on display.

> Would it be possible to include the "text" versions of
> current-indentation and current-column into Emacs?
> 
> Something like
> 
> (defmacro org-current-text-indentation ()
>   "Like `current-indentation', but ignore display/invisible properties."
>   `(let ((buffer-invisibility-spec nil))
>      (current-indentation)))
> 
> (defmacro org-current-text-column ()
>   "Like `current-column', but ignore display/invisible properties."
>   `(string-width (buffer-substring-no-properties
> 		  (line-beginning-position) (point))))

Sounds strange to have this, but if Lars thinks it's useful, I won't
object.

> If not, could you at least mention the invisible text issues in
> `current-indentation' docstring?

OK.

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> A simpler version would be let-binding buffer-invisibility-spec.
>
> Really?

Could you please elaborate? Do I miss something?

I tested let-binding, and it does work if you try to let-bind
buffer-invisibility spec around the calls to `current-indentation' in my
reproducer.

AFAIK, remove-from-invisibility-spec simply does
(setq buffer-invisibility-spec ...)
let-binding is less cumbersome and not limited to outline spec which is
not even used by Org. Org uses multiple specs + other font-lock staff
may set more arbitrary specs in third-party packages.

>> Note that there is similar issue with current-column, though
>> current-column is much less likely to be used without considering
>> display properties.
>
> These are all display-related, in that their results are supposed to
> closely reflect what will be shown on display.

I understand. current-column even express it clearly in the docstring
(unlike, current-indentation). However, as I tried to explain,
display-independent versions can be useful during parsing. And not only
in Org mode.

If you think otherwise, we will simply implement the required versions
in Org.

>> If not, could you at least mention the invisible text issues in
>> `current-indentation' docstring?
>
> OK.

Thanks!

Best,
Ihor

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  56837@debbugs.gnu.org
> Date: Sat, 30 Jul 2022 19:49:50 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> A simpler version would be let-binding buffer-invisibility-spec.
> >
> > Really?
> 
> Could you please elaborate? Do I miss something?

Just that I know very well what let-binding does.

The snippet I sent was copy/pasted from the experiments I performed,
it wasn't supposed to be the peak of elegance.

> > These are all display-related, in that their results are supposed to
> > closely reflect what will be shown on display.
> 
> I understand. current-column even express it clearly in the docstring
> (unlike, current-indentation). However, as I tried to explain,
> display-independent versions can be useful during parsing. And not only
> in Org mode.
> 
> If you think otherwise, we will simply implement the required versions
> in Org.

Let's see what Lars thinks about this.

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Eli Zaretskii]

> Cc: 56837@debbugs.gnu.org
> Date: Sat, 30 Jul 2022 14:38:47 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > If not, could you at least mention the invisible text issues in
> > `current-indentation' docstring?
> 
> OK.

Now done.

bug#56837: 28.1.90; `current-indentation' sometimes returns zero inside invisible text

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> (defmacro org-current-text-indentation ()
>>   "Like `current-indentation', but ignore display/invisible properties."
>>   `(let ((buffer-invisibility-spec nil))
>>      (current-indentation)))
>> 
>> (defmacro org-current-text-column ()
>>   "Like `current-column', but ignore display/invisible properties."
>>   `(string-width (buffer-substring-no-properties
>> 		  (line-beginning-position) (point))))
>
> Sounds strange to have this, but if Lars thinks it's useful, I won't
> object.

It seems quite special-purpose -- I'm not sure there's much general
utility for functions like this.