Re: [PATCH] Fix org-[beginning|end]-of-line with arguments

[Ihor Radchenko <yantar92@posteo.net>]

倉成智久 <tomohisa.kuranari@gmail.com> writes:

> In the current implementation, org-special-ctrl-a/e may not operate as expected.
> For example, executing (org-beginning-of-line 2) relocates the cursor
> to the start of the line, rather than after the heading symbols even
> if org-special-ctrl-a/e is t.
> (Movements to prior lines, such as (org-beginning-of-line 0), function
> correctly.)

> This is my first patch submission. If there are any shortcomings or
> additional requirements needed, please do not hesitate to inform me. I
> am open to feedback and willing to make any necessary adjustments.

Thanks for the patch, and especially for providing tests!
See my comments below.

> Subject: [PATCH] lisp/org.el: Fix the issue with argumented function calls

The above message is not very clear. It would be better if you summarize
the commit purpose more precisely. For example,
"org-beginning/end-of-line: Fix when moving to different line"

> * lisp/org.el (org-beginning-of-line, org-end-of-line): Fix issue with org-special-ctrl-a/e not working correctly when moving with arguments

Please, quote Elisp symbols like `org-special-ctrl-a/e'. Also, we
usually limit the line width to default `fill-column'.

> -	  (when (or (> origin refpos) (= origin bol))
> +	  (when (or (> origin refpos)
> +		    (= origin bol)
> +		    (/= (line-number-at-pos origin) (line-number-at-pos bol)))

This will work, but I am not a big fan of using `line-number-at-pos' -
it is rather slow. May simply check (< origin bol)

> -	  (when (or (> origin after-bullet) (= (point) origin))
> +	  (when (or (> origin after-bullet)
> +		    (= (point) origin)
> +		    (/= (line-number-at-pos origin) (line-number-at-pos (point))))

Same here - (< origin (point))

> -	       (if (or (< origin tags) (= origin (line-end-position)))
> +	       (if (or (< origin tags)
> +		       (= origin (line-end-position))
> +		       (/= (line-number-at-pos origin) (line-number-at-pos (point))))

(> origin (line-end-position))

Finally, your patch is on the edge of legally allowed contribution we are
allowed to accept without copyright assignment. You may consider signing
the copyright form as described in
https://orgmode.org/worg/org-contribute.html#copyright. Or you can add
"TINYCHANGE" to the commit message (see
https://orgmode.org/worg/org-contribute.html#first-patch)

-- 
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>