[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Fix moving cursor in org-set-tags-command
From: |
Matthew Lundin |
Subject: |
Re: [PATCH] Fix moving cursor in org-set-tags-command |
Date: |
Sun, 10 May 2020 12:39:37 -0500 |
Nicolas Goaziou <address@hidden> 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