Ihor Radchenko <yantar92@posteo.net> writes:

> Evgenii Klimov <eugene.dev@lipklim.org> writes:
[...]
> Few comments.
>
> First, please add Changelog entry for etc/ORG-NEWS.

Done.

>> +(defvar org-metaleft-final-hook nil
>> +  "Hook for functions attaching themselves to `M-left'.
>> +See `org-ctrl-c-ctrl-c-hook' for more information.")
>
> Note that unlike the docstring your added, `org-shiftup-final-hook'
> specifies when the hook is executed:
>
> (defvar org-shiftup-final-hook nil
>   "Hook for functions attaching themselves to `S-up'.
> This one runs after all other options except shift-select have been excluded.
> See `org-ctrl-c-ctrl-c-hook' for more information.")
>
> It is better to follow the pattern and clarify the purpose of the hook
> better instead of leaving docstrings for both the -final and other hook
> same.

Done.

>> +individual commands for more information.
>> +
>> +This function runs the hook `org-shiftmetaleft-hook' as a first
>> +step, `org-shiftmetaleft-final-hook' as the penultimate step, and
>> +returns at first non-nil value."
>
> Upon looking closer, I realized that you also defined return value of
> the function here.  Is there any particular reason for this? The return
> value is currently not defined and supposed to be discarded.

> Apparently the original wording is quite confusing.
> What it is supposed to say is:
> 
>     This function runs the functions in `org-metaleft-hook' one by one
>     as a first step, and exits immediately if a function from the hook
>     returns non-nil.

Reworded as we discussed. Also for commands that support
`shift-select-mode' (i.e. org-shift{up,down,right,left}) mentioned it
and `org-support-shift-select'.