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

[Tomohisa Kuranari]

Thank you for your detailed review and the merge of my patch.
I’m so glad to contribute to org-mode!

-- 
Tomohisa Kuranari
Email: tomohisa.kuran...@gmail.com

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

[Ihor Radchenko]

Bastien Guerry  writes:

>>> Finally, the copyright assignment process was completed on 2023/09/20.
>>
>> Bastien, may you please check FSF records?
>
> Done, I confirm Tomohisa Kuranari's record is okay.

Thanks for checking.
Now, I can apply the patch.

Applied, onto bugfix.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=93ebd64de

Tomohisa is also now listed as an Org contributor.
https://git.sr.ht/~bzg/worg/commit/334d3b28

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

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

[Bastien Guerry]

Ihor Radchenko  writes:

> Tomohisa Kuranari  writes:
>
>> Finally, the copyright assignment process was completed on 2023/09/20.
>
> Bastien, may you please check FSF records?

Done, I confirm Tomohisa Kuranari's record is okay.

-- 
 Bastien Guerry

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

[Ihor Radchenko]

Tomohisa Kuranari  writes:

> Finally, the copyright assignment process was completed on 2023/09/20.

Bastien, may you please check FSF records?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

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

[Tomohisa Kuranari]

Hi, Ihor

Thank you for your feedback.
I found your guidance on writing a clearer message, quoting Elisp symbols,
and the insights about indentation rules to be particularly valuable.

Not using line-number-at-pos is truly a great suggestion.
I've reworked the patch and also revised the commit message.
I kindly request another review. If there are any issues, I will make
further revisions.

Finally, the copyright assignment process was completed on 2023/09/20.
Thank you for letting me know.

On Thu, Oct 5, 2023 at 7:03 PM Ihor Radchenko  wrote:
>
> 倉成智久  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 .
> Support Org development at ,
> or support my work at 



-- 
Tomohisa Kuranari
Email: tomohisa.kuran...@gmail.com


0001-org-beginning-end-of-line-Fix-when-moving-to-differe.patch
Description: Binary data

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

[Ihor Radchenko]

倉成智久  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 .
Support Org development at ,
or support my work at