Re: [PATCH] Fix moving cursor in org-set-tags-command

[Matthew Lundin <mdl@imapmail.org>]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>
>> -    (when (save-excursion (skip-chars-backward "*") (bolp))
>> -      (forward-char))))
>> +    (and (looking-at " ")
>> +	 (string-match "\\*+" (buffer-substring (point-at-bol) (point)))
>> +	 (forward-char))))
>
> Please replace `and' with `when' if side-effects are involved.
>

Thanks, this is really helpful to know for future patches. And thanks so
much for the fixes you and Kyle made.

> Also, note that
>
>   (save-excursion (skip-chars-backward "*") (bolp))
>
> is faster and more accurate than
>
>   (string-match "\\*+" (buffer-substring (point-at-bol) (point)))
>
> because the latter matches, e.g.,
>
>   ab*|c
>
> where "|" is point.

Oops, yes I see the problem in the string-match there!

> Besides, I don't understand how this is related to empty headlines
> since, AFAICT, this part of code is supposed to fix the issue on empty
> headlines.

Thanks. What I meant was fixing a very specific circumstance "when the
cursor is at the beginning of an empty headline." This is the scenario
affected by the original bug. The earlier patches (450452de4b and
44ec473c1) introduced a more general logic of moving the cursor forward
at any point before the beginning of an empty headline (including when
positioned on an asterisk). I see the commit has already been made, so
hopefully this email will serve as clarification.

Best,

Matt