manual: stale bindings for org-clock-report and org-columns-insert-dblock

manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Kyle Meyer]

Hello,

34b71a0ca (Add a dispatcher command for inserting dynamic blocks,
2018-12-23) removed the key bindings of org-clock-report and
org-columns-insert-dblock, but these bindings are still listed in
org-manual.org.  stardiviner, could you please have a look at updating
the manual to fully capture the changes from 34b71a0ca?  I think it'd
also be good to mention these removed key bindings in ORG-NEWS.

Thanks.

-- 
Kyle

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[stardiviner]

Kyle Meyer <kyle@kyleam.com> writes:

> Hello,
>
> 34b71a0ca (Add a dispatcher command for inserting dynamic blocks,
> 2018-12-23) removed the key bindings of org-clock-report and
> org-columns-insert-dblock, but these bindings are still listed in
> org-manual.org.  stardiviner, could you please have a look at updating
> the manual to fully capture the changes from 34b71a0ca?  I think it'd
> also be good to mention these removed key bindings in ORG-NEWS.
>
> Thanks.

You're right, I have not noticed them. I will add a patch to submit
soon. Thanks for your mention, Kyle.

-- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
      

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nikolay Kudryavtsev]

Am I reading it correctly that C-c C-x C-r is no longer a valid 
keybinding? Is this the reason why it now behaves so stupidly, creating 
a block with :scope file and asking for that file? Can someone point me 
to a discussion of that change? Manual is still not updated about it and 
nothing in ORG-NEWS.

Quickly using that key was quite useful to pop a clock report for a task 
and I really don't see why would anyone want to remove that.

-- 
Best Regards,
Nikolay Kudryavtsev

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Kyle Meyer]

Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com> writes:

> Am I reading it correctly that C-c C-x C-r is no longer a valid
> keybinding?

Yes, the binding was removed in 34b71a0ca (Add a dispatcher command for
inserting dynamic blocks, 2018-12-23).

> Is this the reason why it now behaves so stupidly, creating
> a block with :scope file and asking for that file?

[You could have phrased that less abrasively.]

I don't know.  I don't use org-clock-report, haven't looked closely at
34b71a0ca, and didn't follow the discussion around the change, but ...

> Can someone point me to a discussion of that change?

I think the relevant discussion started with 871s7lvn4n.fsf@gmail.com

  * https://lists.gnu.org/archive/html/emacs-orgmode/2018-11/msg00183.html
  * https://lists.gnu.org/archive/html/emacs-orgmode/2018-12/msg00026.html

-- 
Kyle

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nikolay Kudryavtsev]

Thanks for the tip.

After reading that discussion and a few hours of research, there are 
actually three different problems I have with org-clock-report.

The first is that drop in clock reports now use :scope file by default, 
instead of :scope subtree.

The second is that the clock report function is set up in some weird way 
that it gets broken if you use icicles. Gonna look into it further, not 
sure whether org or icicles is the bad actor here. The behavior I'm 
seeing is as follows:

The command spawns empty table with :scope file, asks for a file, when I 
select that file, it asks for a function and I have no idea which 
function should I choose here.

For those two problems I've bisected the offending commit to 1a5151c51.

The third is the removal of C-c C-x C-r. I've read the discussion there, 
but it seems like nobody really wanted it removed, this is just a  
consequence of refactoring. So, can we get C-c C-x C-r org-clock-report 
back, if only as a closure over org-dynamic-block-insert-dblock?

Also, ever since that Stardiviner's commit the dynamic reloading of org 
seems to be broken on master. When I try to I get:

Symbol’s function definition is void: org-dynamic-block-define

Another thing, since org is moving towards unifying dynamic blocks, 
maybe the next step is to add begin_src blocks into the mix in 
org-dynamic-block-insert-dblock? It seems that they historically have 
different syntax, but are in practice the same thing. That's actually 
one place where I could use a wizard.

And why the function is called org-dynamic-block-insert-dblock and not 
org-insert-dblock? Seems redundant to me.

Also can someone explain how do I get from org elpa version to the git 
commit it's based on? E. g., in org-version.el I have 
"9.2.1-33-g029cf6-elpaplus" but g029cf6 does not seem like a real commit 
in the git repo.

-- 
Best Regards,
Nikolay Kudryavtsev

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nicolas Goaziou]

Hello,

Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com> writes:

> The first is that drop in clock reports now use :scope file by
> default, instead of :scope subtree.

Indeed. This is because clocktable default properties include :scope
property. I removed it. Thank you.

> The second is that the clock report function is set up in some weird
> way that it gets broken if you use icicles. Gonna look into it
> further, not sure whether org or icicles is the bad actor here. The
> behavior I'm seeing is as follows:
>
> The command spawns empty table with :scope file, asks for a file, when
> I select that file, it asks for a function and I have no idea which
> function should I choose here.

I don't use Icicles, but I cannot reproduce it. What are the exact steps
to do so?

> The third is the removal of C-c C-x C-r. I've read the discussion
> there, but it seems like nobody really wanted it removed, this is just
> a  consequence of refactoring. So, can we get C-c C-x C-r
> org-clock-report back, if only as a closure over
> org-dynamic-block-insert-dblock?

I don't see the need for that. You can, however, bind `C-c C-x C-r' to
`org-clock-report' in your config.

> Also, ever since that Stardiviner's commit the dynamic reloading of
> org seems to be broken on master. When I try to I get:
>
> Symbol’s function definition is void: org-dynamic-block-define

You seem to have a mixed installation. This function only exists in
development branch, i.e., Org 9.3, not ELPA.

> Another thing, since org is moving towards unifying dynamic blocks,
> maybe the next step is to add begin_src blocks into the mix in
> org-dynamic-block-insert-dblock? It seems that they historically have
> different syntax, but are in practice the same thing. That's actually
> one place where I could use a wizard.

My practice differs. I don't see them as the same thing. Their syntax is
vaguely related, but that's all.

Note you can insert source blocks with 'C-c C-,`

> And why the function is called org-dynamic-block-insert-dblock and not
> org-insert-dblock? Seems redundant to me.

Hysterical raisins, I assume.

> Also can someone explain how do I get from org elpa version to the git
> commit it's based on? E. g., in org-version.el I have
> "9.2.1-33-g029cf6-elpaplus" but g029cf6 does not seem like a real
> commit in the git repo.

Drop the "g". The relative commit is "029cf6".

Regards,

-- 
Nicolas Goaziou

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nikolay Kudryavtsev]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

> I don't use Icicles, but I cannot reproduce it. What are the exact steps
> to do so?
Sorry, it's not clear whether you use Icicles. At the moment the recipe is:

1. Install icicles as described here 
<https://www.emacswiki.org/emacs/Icicles_-_Libraries>. Enable it by 
doing icy-mode.

2. Do org-clock-report.

I'll try to see whether there's some other way this odd behavior shows 
itself, that can be seen without using Icicles. At the moment, do you 
see anything in that commit that made this command, let's say abnormal?

> I don't see the need for that. You can, however, bind `C-c C-x C-r' to
> `org-clock-report' in your config.
Minor thing, but I still see this as a regression, without a good 
reason, but it's your call...

> You seem to have a mixed installation. This function only exists in
> development branch, i.e., Org 9.3, not ELPA.
I'll double check and report.

> Note you can insert source blocks with 'C-c C-,`
Didn't knew that, thanks.

> Drop the "g". The relative commit is "029cf6".
Thanks.

-- 
Best Regards,
Nikolay Kudryavtsev


[-- Attachment #2: Type: text/html, Size: 1952 bytes --]

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nikolay Kudryavtsev]

I was able to debug the Icicles interaction bug:

In org-dblock-write:clocktable there's a check ((pred functionp) 
(funcall scope)) which checks whether the scope is a function. Since the 
default scope is currently "file", it gets funcalled. Icicles provides a 
function called file. There it can be disabled by setting (setq 
icicle-define-alias-commands-flag nil), but getting to that would 
generally mean that the user ran into some problems and did the same 
debugging I had to do.

I'm not 100% sure but I'd rather prefer org not calling any functions 
with generic names like "file" or "subtree", so maybe prepending some 
predicate on a call is warranted, so instead of "file", something like 
"org-scope-file" would get checked, but I don't really understand the 
intended use of this feature.

As for dynamic reloading, I retested and can confirm that it's broken 
since 69c300bbf. To repeat, start with elpa org, add master org to the 
load path, do org-reload.

-- 
Best Regards,
Nikolay Kudryavtsev

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nicolas Goaziou]

Hello,

Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com> writes:

> I was able to debug the Icicles interaction bug:
>
> In org-dblock-write:clocktable there's a check ((pred functionp)
> (funcall scope)) which checks whether the scope is a function. Since
> the default scope is currently "file", it gets funcalled. Icicles
> provides a function called file. There it can be disabled by setting
> (setq icicle-define-alias-commands-flag nil), but getting to that
> would generally mean that the user ran into some problems and did the
> same debugging I had to do.

Icicles is doing it wrong. Packages are supposed to provide only
prefixed functions, since there is no namespace in Emacs.

> I'm not 100% sure but I'd rather prefer org not calling any functions
> with generic names like "file" or "subtree", so maybe prepending some
> predicate on a call is warranted, so instead of "file", something like
> "org-scope-file" would get checked, but I don't really understand the
> intended use of this feature.

I pushed a different change. Tell me if it fixes your issue.

> As for dynamic reloading, I retested and can confirm that it's broken
> since 69c300bbf. To repeat, start with elpa org, add master org to the
> load path, do org-reload.

So, you are doing a mixed installation on purpose. No wonder you have
troubles. Don't reload a different installation on top of a loaded one.

Regards,

-- 
Nicolas Goaziou

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nikolay Kudryavtsev]

Thanks, everything's fine now. Sorry for late reply.

> Icicles is doing it wrong. Packages are supposed to provide only
> prefixed functions, since there is no namespace in Emacs.
As far as I could grasp, those are shorthand alias functions created for 
easier access. Luckily that behavior can be disabled by a variable. And 
they're probably enabled by default because nobody complained yet.

> So, you are doing a mixed installation on purpose. No wonder you have
> troubles. Don't reload a different installation on top of a loaded one.
Yes, wishful thinking on my part. Been doing that for ages and the 
reason I didn't ran into many issues is that I was usually reloading 
versions that are pretty close, while bisecting for bugs. In my defense 
I assumed that org-reload does some powerful unload magic. And it's 
definitely a kind of magic that I would be scared to even look at, so it 
never crossed my mind to look into it's sources.;-)

-- 
Best Regards,
Nikolay Kudryavtsev

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[Nick Dokos]

Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com> writes:

> ...
> Also can someone explain how do I get from org elpa version to the git
> commit it's based on? E. g., in org-version.el I have
> "9.2.1-33-g029cf6-elpaplus" but g029cf6 does not seem like a real
> commit in the git repo.

Try it without the 'g': that's just a prefix. The commit is 029cf6.

-- 
Nick

"There are only two hard problems in computer science: cache
invalidation, naming things, and off-by-one errors." -Martin Fowler

Re: manual: stale bindings for org-clock-report and org-columns-insert-dblock

[stardiviner]

Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com> writes:

> Am I reading it correctly that C-c C-x C-r is no longer a valid keybinding? Is
> this the reason why it now behaves so stupidly, creating a block with :scope
> file and asking for that file? Can someone point me to a discussion of that
> change? Manual is still not updated about it and nothing in ORG-NEWS.
>
> Quickly using that key was quite useful to pop a clock report for a task and I
> really don't see why would anyone want to remove that.

I only added dispatcher for commands, but not modify command functions itself.
If I remember correctly, that commit indeed added changes in ORG-NEWS. So...

That's all I can say. I suggestion you check out Git log history with blame, to
figure what change that command behavior.

And don't be rude to people.

-- 
[ stardiviner ]
       I try to make every word tell the meaning what I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3