emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Missing argument in org-reset-checkbox-state-subtree?
@ 2020-11-11 17:37 Bob Wilson
  2020-11-12  3:22 ` Kyle Meyer
  0 siblings, 1 reply; 5+ messages in thread
From: Bob Wilson @ 2020-11-11 17:37 UTC (permalink / raw)
  To: emacs-orgmode

Hi folks,
   I’m writing about a possible bug in org-reset-checkbox-state-subtree. This function calls org-update-checkbox-count-maybe with argument 'all, but this value is not defined in the function (or anywhere else that I can see). 

I’d like this value to be nil because I don’t want to update the statistics cookies in the entire buffer, but I don’t see a way to do this (pardon my limited elisp). My workaround is to add an optional all argument to org-reset-checkbox-state-subtree and pass it to org-update-checkbox-count-maybe.

This seems like the desired functionality unless I’m missing something. Is the current implementation intentional?

Kind regards,
Bob Wilson

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Missing argument in org-reset-checkbox-state-subtree?
  2020-11-11 17:37 Missing argument in org-reset-checkbox-state-subtree? Bob Wilson
@ 2020-11-12  3:22 ` Kyle Meyer
  2020-11-12 16:59   ` Bob Wilson
  2020-11-12 23:38   ` Bob Wilson
  0 siblings, 2 replies; 5+ messages in thread
From: Kyle Meyer @ 2020-11-12  3:22 UTC (permalink / raw)
  To: Bob Wilson; +Cc: emacs-orgmode

Bob Wilson writes:

> I’m writing about a possible bug in
> org-reset-checkbox-state-subtree. This function calls
> org-update-checkbox-count-maybe with argument 'all, but this value is
> not defined in the function (or anywhere else that I can see).

[ I'm looking at the copy on master (e9c3993ee), though org-list.el
  hasn't changed since that last release. ]

org-reset-checkbox-state-subtree calls

    (org-update-checkbox-count-maybe 'all)

and org-update-checkbox-count-maybe calls

    (org-update-checkbox-count all)

And org-update-checkbox-count considers ALL, so I'm not spotting
anything that's undefined.

> I’d like this value to be nil because I don’t want to update the
> statistics cookies in the entire buffer, but I don’t see a way to do
> this (pardon my limited elisp). My workaround is to add an optional
> all argument to org-reset-checkbox-state-subtree and pass it to
> org-update-checkbox-count-maybe.

Why does refreshing the stats for the entire buffer cause a problem for
you?

> This seems like the desired functionality unless I’m missing
> something. Is the current implementation intentional?

Yes, the change was made with a0bc3bdeb (org-list: fix update of
check-boxes cookies in whole trees, 2011-06-20).  The example that
prompted that change was reported at
<https://orgmode.org/list/87fwn4bhcy.fsf@gmail.com/>.

Here's a reduced example:

  * a
  ** aa [1/1]
  - [X] l
  ** ab [1/1]
  - [X] m

Calling org-reset-checkbox-state-subtree with point at the top-level "a"
should uncheck l and m and the stats of both subheadings should go to
[0/1]:

  * a
  ** aa [0/1]
  - [ ] l
  ** ab [0/1]
  - [ ] m

If you were to drop `all' from -reset-checkbox-state-subtree's the call
to -update-checkbox-count-maybe, the result would instead be

  * a
  ** aa [1/1]
  - [ ] l
  ** ab [0/1]
  - [ ] m


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Missing argument in org-reset-checkbox-state-subtree?
  2020-11-12  3:22 ` Kyle Meyer
@ 2020-11-12 16:59   ` Bob Wilson
  2020-11-12 23:10     ` Kyle Meyer
  2020-11-12 23:38   ` Bob Wilson
  1 sibling, 1 reply; 5+ messages in thread
From: Bob Wilson @ 2020-11-12 16:59 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: emacs-orgmode

Hi Kyle,
   Thanks so much for your thoughtful response. It would probably be helpful if I gave more context on the problem I was having!

I have a daily wind-down routine where I respond to any lingering emails, etc. So I have an item like:

* TODO Daily Wind-Down
SCHEDULED: <2020-11-12 Thu 17:00 +1d>
- [ ] Respond to emails
- [ ] Pat myself on the back

I use the org-checklist add-on so that marking the task as complete resets the checkboxes to empty. But I noticed that every time I do so, it resets the statistics cookies everywhere else in the org file! And by reset, I mean it replaces things like [5/7] with [0/0]. I have to do C-u C-c # (org-update-statistics-cookies ALL) to fix it.

I’m new to elisp, but I tried my best to debug what was going on. I could be completely wrong about the underlying cause, but I was able to make a change that prevented this phenomenon from occurring. I thought other people might be having the same problem, but it sounds like maybe not!

Just for completeness, I will write out my thoughts (and you’ll soon see just how little I know of elisp — I apologize!). Here is the org-reset-checkbox-state-subtree as of 9.4:

(defun org-reset-checkbox-state-subtree ()
  "Reset all checkboxes in an entry subtree."
  (interactive "*")
  (if (org-before-first-heading-p)
      (error "Not inside a tree")
    (save-restriction
      (save-excursion
	(org-narrow-to-subtree)
	(org-show-subtree)
	(goto-char (point-min))
	(let ((end (point-max)))
	  (while (< (point) end)
	    (when (org-at-item-checkbox-p)
	      (replace-match "[ ]" t t nil 1))
	    (beginning-of-line 2)))
	(org-update-checkbox-count-maybe 'all)))))

In the last line, it calls org-update-checkbox-count-maybe with ‘all argument. I don’t full understand the single quote part. My understanding is that it tells lisp not to evaluate the value (which is good, because there is no all variable defined at this scope). My only idea here is that it’s a way of saying, “by the time you actually need this value, I’ll have told you what it is”. But what is relevant is that I don’t think ‘all is nil; it’s some kind of placeholder, so it effectively gets evaluated as true. Or so it seems to me.

org-update-checkbox-count-maybe passes this placeholder unmodified to org-update-checkbox-count which does all the heavy lifting. But nowhere is the all argument ever modified, it continues to be that non-nil placeholder, which seems to get evaluated as true. It’s not clear to me why this placeholder would get passed to the function if it is never set to anything else. But again, I don’t fully understand the single quote syntax.

The org-update-checkbox-count is quite complicated (from my elisp-neophyte perspective), so it is not clear to me why it resets the statistics cookies to [0/0], but whatever it does, it does it in the entire file.

The “fix” that I made was on the first and last lines of the function:

(defun org-reset-checkbox-state-subtree (&optional all)
...
	(org-update-checkbox-count-maybe all)))))

where you can see all I did was remove the single quote from all, and added all as an optional parameter. Now I feel like this was the wrong fix, but it does solve my problem! Maybe the real problem had nothing to do with the entire buffer part, and was more about why the statistics cookies get updated to [0/0].

Here is a minimal example that showcases the problem (without my “fix” in place):

* Task A [1/2]
** DONE Subtask A.1
   CLOSED: [2020-11-12 Thu 08:47]
** TODO Subtask A.2
* TODO Daily Wind-Down
  SCHEDULED: <2020-11-12 Thu 17:00 +1d>
  :PROPERTIES:
  :RESET_CHECK_BOXES: t
  :END:
- [ ] Respond to emails
- [ ] Pat myself on the back

Marking the Daily Wind-Down as complete resets the buffer so it looks like:
* Task A [0/0]
** DONE Subtask A.1
   CLOSED: [2020-11-12 Thu 08:47]
** TODO Subtask A.2
* TODO Daily Wind-Down
  SCHEDULED: <2020-11-13 Fri 17:00 +1d>
  :PROPERTIES:
  :RESET_CHECK_BOXES: t
  :LAST_REPEAT: [2020-11-12 Thu 08:47]  
  :END:
- [ ] Respond to emails
- [ ] Pat myself on the back

The first line shows the problem: the statistics cookies have updated to [0/0].

For what it’s worth, my “fix” may be the wrong fix, but it does solve the problem! So if anyone else runs into the same problem, I hope my horrible elisp debugging is mildly helpful!

Kind regards,
Bob Wilson


> On Nov 11, 2020, at 7:22 PM, Kyle Meyer <kyle@kyleam.com> wrote:
> 
> Bob Wilson writes:
> 
>> I’m writing about a possible bug in
>> org-reset-checkbox-state-subtree. This function calls
>> org-update-checkbox-count-maybe with argument 'all, but this value is
>> not defined in the function (or anywhere else that I can see).
> 
> [ I'm looking at the copy on master (e9c3993ee), though org-list.el
>  hasn't changed since that last release. ]
> 
> org-reset-checkbox-state-subtree calls
> 
>    (org-update-checkbox-count-maybe 'all)
> 
> and org-update-checkbox-count-maybe calls
> 
>    (org-update-checkbox-count all)
> 
> And org-update-checkbox-count considers ALL, so I'm not spotting
> anything that's undefined.
> 
>> I’d like this value to be nil because I don’t want to update the
>> statistics cookies in the entire buffer, but I don’t see a way to do
>> this (pardon my limited elisp). My workaround is to add an optional
>> all argument to org-reset-checkbox-state-subtree and pass it to
>> org-update-checkbox-count-maybe.
> 
> Why does refreshing the stats for the entire buffer cause a problem for
> you?
> 
>> This seems like the desired functionality unless I’m missing
>> something. Is the current implementation intentional?
> 
> Yes, the change was made with a0bc3bdeb (org-list: fix update of
> check-boxes cookies in whole trees, 2011-06-20).  The example that
> prompted that change was reported at
> <https://orgmode.org/list/87fwn4bhcy.fsf@gmail.com/>.
> 
> Here's a reduced example:
> 
>  * a
>  ** aa [1/1]
>  - [X] l
>  ** ab [1/1]
>  - [X] m
> 
> Calling org-reset-checkbox-state-subtree with point at the top-level "a"
> should uncheck l and m and the stats of both subheadings should go to
> [0/1]:
> 
>  * a
>  ** aa [0/1]
>  - [ ] l
>  ** ab [0/1]
>  - [ ] m
> 
> If you were to drop `all' from -reset-checkbox-state-subtree's the call
> to -update-checkbox-count-maybe, the result would instead be
> 
>  * a
>  ** aa [1/1]
>  - [ ] l
>  ** ab [0/1]
>  - [ ] m



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Missing argument in org-reset-checkbox-state-subtree?
  2020-11-12 16:59   ` Bob Wilson
@ 2020-11-12 23:10     ` Kyle Meyer
  0 siblings, 0 replies; 5+ messages in thread
From: Kyle Meyer @ 2020-11-12 23:10 UTC (permalink / raw)
  To: Bob Wilson; +Cc: emacs-orgmode

Bob Wilson writes:

> I use the org-checklist add-on so that marking the task as complete
> resets the checkboxes to empty. But I noticed that every time I do so,
> it resets the statistics cookies everywhere else in the org file! And
> by reset, I mean it replaces things like [5/7] with [0/0]. I have to
> do C-u C-c # (org-update-statistics-cookies ALL) to fix it.

[ Rearranging things to lead with your complete example first... ]

> * Task A [1/2]
> ** DONE Subtask A.1
>    CLOSED: [2020-11-12 Thu 08:47]
> ** TODO Subtask A.2
> * TODO Daily Wind-Down
>   SCHEDULED: <2020-11-12 Thu 17:00 +1d>
>   :PROPERTIES:
>   :RESET_CHECK_BOXES: t
>   :END:
> - [ ] Respond to emails
> - [ ] Pat myself on the back
>
> Marking the Daily Wind-Down as complete resets the buffer so it looks like:
> * Task A [0/0]
> ** DONE Subtask A.1
>    CLOSED: [2020-11-12 Thu 08:47]
> ** TODO Subtask A.2
> * TODO Daily Wind-Down
>   SCHEDULED: <2020-11-13 Fri 17:00 +1d>
>   :PROPERTIES:
>   :RESET_CHECK_BOXES: t
>   :LAST_REPEAT: [2020-11-12 Thu 08:47]  
>   :END:
> - [ ] Respond to emails
> - [ ] Pat myself on the back
>
> The first line shows the problem: the statistics cookies have updated
> to [0/0].

Thanks.  I can trigger that as well.  So, with the example from
<https://orgmode.org/list/87fwn4bhcy.fsf@gmail.com/>, the issue is that,
without specifying `all', the _checkbox_ statistics for sibling
subheadings do not get updated, just the last one.

In contrast, the problem here looks to be that org-update-checkbox-count
will result in _todo_ statistics being set to zero, and it doesn't take
care of refreshing them.

This problem goes away with the higher-level
org-update-statistics-cookies because it calls
(org-update-checkbox-count 'all) but _then_ follows up with a call to
(org-map-entries 'org-update-parent-todo-statistics).

> Just for completeness, I will write out my thoughts (and you’ll soon
> see just how little I know of elisp — I apologize!). Here is the
> org-reset-checkbox-state-subtree as of 9.4:
>
> (defun org-reset-checkbox-state-subtree ()
[...]
> 	(org-update-checkbox-count-maybe 'all)))))
>
> In the last line, it calls org-update-checkbox-count-maybe with ‘all
> argument. I don’t full understand the single quote part. My
> understanding is that it tells lisp not to evaluate the value (which
> is good, because there is no all variable defined at this scope).

Yes, that's right.  (info "(elisp)Quoting") has more details.

> My only idea here is that it’s a way of saying, “by the time you
> actually need this value, I’ll have told you what it is”. But what is
> relevant is that I don’t think ‘all is nil; it’s some kind of
> placeholder, so it effectively gets evaluated as true. Or so it seems
> to me.
>
> org-update-checkbox-count-maybe passes this placeholder unmodified to
> org-update-checkbox-count which does all the heavy lifting. But
> nowhere is the all argument ever modified, it continues to be that
> non-nil placeholder, which seems to get evaluated as true. It’s not
> clear to me why this placeholder would get passed to the function if
> it is never set to anything else. But again, I don’t fully understand
> the single quote syntax.

In this particular case, org-update-checkbox-count doesn't check for a
_specific_ non-nil value, so indeed the only thing that is important is
that the value of org-update-checkbox-count's ALL ends up non-nil.  t
(no need to quote), or any number of things aside from nil, could be
passed instead.

Using "'all" rather than "t" here is just making things a bit more
readable by hinting what the parameter/effect is.

> The org-update-checkbox-count is quite complicated (from my
> elisp-neophyte perspective), so it is not clear to me why it resets
> the statistics cookies to [0/0], but whatever it does, it does it in
> the entire file.
>
> The “fix” that I made was on the first and last lines of the function:
>
> (defun org-reset-checkbox-state-subtree (&optional all)
> ...
> 	(org-update-checkbox-count-maybe all)))))
>
> where you can see all I did was remove the single quote from all, and
> added all as an optional parameter. Now I feel like this was the wrong
> fix, but it does solve my problem!

Right, it's problematic because it'd bring back the previous issue with
sibling checkbox statistics not being updated.

> Maybe the real problem had nothing to do with the entire buffer part,
> and was more about why the statistics cookies get updated to [0/0].

That's my current thinking, but I haven't taken a closer look at
org-update-checkbox-count yet.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Missing argument in org-reset-checkbox-state-subtree?
  2020-11-12  3:22 ` Kyle Meyer
  2020-11-12 16:59   ` Bob Wilson
@ 2020-11-12 23:38   ` Bob Wilson
  1 sibling, 0 replies; 5+ messages in thread
From: Bob Wilson @ 2020-11-12 23:38 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: emacs-orgmode

Thanks again Kyle,
   Per your suggestion I have reverted my change to org-reset-checkbox-state-subtree, and instead updated org-reset-checkbox-state-maybe in org-checklist.el (I’m much happier editing org add-ons than editing org!).

Specifically, I follow up org-reset-checkbox-state-subtree with a call to:
(org-map-entries ‘org-update-parent-todo-statistics)

and this seems to address my problem as well, but definitely more elegantly!

Kind regards,
Bob

> On Nov 11, 2020, at 7:22 PM, Kyle Meyer <kyle@kyleam.com> wrote:
> 
> Bob Wilson writes:
> 
>> I’m writing about a possible bug in
>> org-reset-checkbox-state-subtree. This function calls
>> org-update-checkbox-count-maybe with argument 'all, but this value is
>> not defined in the function (or anywhere else that I can see).
> 
> [ I'm looking at the copy on master (e9c3993ee), though org-list.el
>  hasn't changed since that last release. ]
> 
> org-reset-checkbox-state-subtree calls
> 
>    (org-update-checkbox-count-maybe 'all)
> 
> and org-update-checkbox-count-maybe calls
> 
>    (org-update-checkbox-count all)
> 
> And org-update-checkbox-count considers ALL, so I'm not spotting
> anything that's undefined.
> 
>> I’d like this value to be nil because I don’t want to update the
>> statistics cookies in the entire buffer, but I don’t see a way to do
>> this (pardon my limited elisp). My workaround is to add an optional
>> all argument to org-reset-checkbox-state-subtree and pass it to
>> org-update-checkbox-count-maybe.
> 
> Why does refreshing the stats for the entire buffer cause a problem for
> you?
> 
>> This seems like the desired functionality unless I’m missing
>> something. Is the current implementation intentional?
> 
> Yes, the change was made with a0bc3bdeb (org-list: fix update of
> check-boxes cookies in whole trees, 2011-06-20).  The example that
> prompted that change was reported at
> <https://orgmode.org/list/87fwn4bhcy.fsf@gmail.com/>.
> 
> Here's a reduced example:
> 
>  * a
>  ** aa [1/1]
>  - [X] l
>  ** ab [1/1]
>  - [X] m
> 
> Calling org-reset-checkbox-state-subtree with point at the top-level "a"
> should uncheck l and m and the stats of both subheadings should go to
> [0/1]:
> 
>  * a
>  ** aa [0/1]
>  - [ ] l
>  ** ab [0/1]
>  - [ ] m
> 
> If you were to drop `all' from -reset-checkbox-state-subtree's the call
> to -update-checkbox-count-maybe, the result would instead be
> 
>  * a
>  ** aa [1/1]
>  - [ ] l
>  ** ab [0/1]
>  - [ ] m



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-11-12 23:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 17:37 Missing argument in org-reset-checkbox-state-subtree? Bob Wilson
2020-11-12  3:22 ` Kyle Meyer
2020-11-12 16:59   ` Bob Wilson
2020-11-12 23:10     ` Kyle Meyer
2020-11-12 23:38   ` Bob Wilson

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).