How about lifting the limit of 35 tasks in org-clock-history?

How about lifting the limit of 35 tasks in org-clock-history?

[Marcin Borkowski]

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

Hi all,

I attach a trivial patch fixing a very annoying cap on
org-clock-history.  (I want to set org-clock-history to 120.)

Best,

--
Marcin Borkowski
http://mbork.pl

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Lift-the-artificial-limit-on-the-clock-history-lengt.patch --]
[-- Type: text/x-patch; size=1.33KiB, Size: 1361 bytes --]

From 10de3a12ebbad820fb86dc6924f17d701d4f9620 Mon Sep 17 00:00:00 2001
From: Marcin Borkowski <mbork@mbork.pl>
Date: Fri, 31 Aug 2018 07:53:42 +0200
Subject: [PATCH] Lift the artificial limit on the clock history length.

The default limit of 35 was hard-coded, and was especially annoying
when using an alternative way of selecting from history,
e.g. https://github.com/unhammer/org-mru-clock.
---
 lisp/org-clock.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 9819fcff2..370392947 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -156,7 +156,9 @@ state to switch it to."
 	  (symbol :tag "Function")))
 
 (defcustom org-clock-history-length 5
-  "Number of clock tasks to remember in history."
+  "Number of clock tasks to remember in history.
+Clocking in using history may not work if this number is higher
+than 35."
   :group 'org-clock
   :type 'integer)
 
@@ -534,7 +536,7 @@ cannot be translated."
 
 (defun org-clock-history-push (&optional pos buffer)
   "Push a marker to the clock history."
-  (setq org-clock-history-length (max 1 (min 35 org-clock-history-length)))
+  (setq org-clock-history-length (max 1 org-clock-history-length))
   (let ((m (move-marker (make-marker)
 			(or pos (point)) (org-base-buffer
 					  (or buffer (current-buffer)))))
-- 
2.18.0

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Nicolas Goaziou]

Hello,

Marcin Borkowski <mbork@mbork.pl> writes:

> I attach a trivial patch fixing a very annoying cap on
> org-clock-history.  (I want to set org-clock-history to 120.)

According to 7972356d092736af06282a09f008c2b5f938134d and
77f1f31c99f521751dc0b57f9923773e97644bb5, this limits selection to
standard selection keys. 

If we want to allow more clock history, we need to remove the selection
keys, and be sure we have something else to use.

WDYT?

Regards,

-- 
Nicolas Goaziou

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Marcin Borkowski]

On 2018-09-02, at 14:45, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> Marcin Borkowski <mbork@mbork.pl> writes:
>
>> I attach a trivial patch fixing a very annoying cap on
>> org-clock-history.  (I want to set org-clock-history to 120.)
>
> According to 7972356d092736af06282a09f008c2b5f938134d and
> 77f1f31c99f521751dc0b57f9923773e97644bb5, this limits selection to
> standard selection keys. 
>
> If we want to allow more clock history, we need to remove the selection
> keys, and be sure we have something else to use.

I decided to put a warning about this in the docstring in my patch.  My
assumption was that this is enough.  If a user wants to change the
default, he will most probably see the docstring and will have to
actively ignore the warning.

I do not use the vanilla Org way of selecting tasks to clock from
history - I use org-mru-clock, for which the restriction doesn't make
sense.  So we have something else to use, and it doesn't bother me that
I can't select older tasks with the original Org method.

Best,

-- 
Marcin Borkowski
http://mbork.pl

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Marcin Borkowski]

On 2018-09-02, at 16:50, Marcin Borkowski <mbork@mbork.pl> wrote:

> On 2018-09-02, at 14:45, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
>
>> Hello,
>>
>> Marcin Borkowski <mbork@mbork.pl> writes:
>>
>>> I attach a trivial patch fixing a very annoying cap on
>>> org-clock-history.  (I want to set org-clock-history to 120.)
>>
>> According to 7972356d092736af06282a09f008c2b5f938134d and
>> 77f1f31c99f521751dc0b57f9923773e97644bb5, this limits selection to
>> standard selection keys. 
>>
>> If we want to allow more clock history, we need to remove the selection
>> keys, and be sure we have something else to use.
>
> I decided to put a warning about this in the docstring in my patch.  My
> assumption was that this is enough.  If a user wants to change the
> default, he will most probably see the docstring and will have to
> actively ignore the warning.
>
> I do not use the vanilla Org way of selecting tasks to clock from
> history - I use org-mru-clock, for which the restriction doesn't make
> sense.  So we have something else to use, and it doesn't bother me that
> I can't select older tasks with the original Org method.

Hi Nicolas and others,

any comments on this patch?

Best,

-- 
Marcin Borkowski
http://mbork.pl

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Nicolas Goaziou]

Hello,

Marcin Borkowski <mbork@mbork.pl> writes:

> I decided to put a warning about this in the docstring in my patch.  My
> assumption was that this is enough.  If a user wants to change the
> default, he will most probably see the docstring and will have to
> actively ignore the warning.

I don't think this is enough. As you put it, it instills doubt without
explaining anything. Why 35? Why "may not work"? What "may not work"?

> I do not use the vanilla Org way of selecting tasks to clock from
> history - I use org-mru-clock, for which the restriction doesn't make
> sense.  So we have something else to use,

I meant "have something else to use out of the box". I.e., not with an
external library.

I agree with the idea of removing this limitation, but I'm not convinced
your patch is ideal yet.

Regards,

-- 
Nicolas Goaziou

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Marcin Borkowski]

On 2018-09-06, at 16:18, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> Marcin Borkowski <mbork@mbork.pl> writes:
>
>> I decided to put a warning about this in the docstring in my patch.  My
>> assumption was that this is enough.  If a user wants to change the
>> default, he will most probably see the docstring and will have to
>> actively ignore the warning.
>
> I don't think this is enough. As you put it, it instills doubt without
> explaining anything. Why 35? Why "may not work"? What "may not work"?

OK.  How about

1. making the docstring more precise

2. leaving the limit, but instead of hard-coded 35 make it configurable
using a variable?

>> I do not use the vanilla Org way of selecting tasks to clock from
>> history - I use org-mru-clock, for which the restriction doesn't make
>> sense.  So we have something else to use,
>
> I meant "have something else to use out of the box". I.e., not with an
> external library.

I see.

Frankly, I think the out-of-the-box way of selecting tasks from history
is not very good.  For starters, it won't work with completion (like
org-mru-clock does).  Also, I think the window with the list in not
scrollable.

> I agree with the idea of removing this limitation, but I'm not convinced
> your patch is ideal yet.

Thanks.  I'll submit a better one soon.

-- 
Marcin Borkowski
http://mbork.pl

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Marcin Borkowski]

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


On 2018-09-06, at 16:18, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> Marcin Borkowski <mbork@mbork.pl> writes:
>
>> I decided to put a warning about this in the docstring in my patch.  My
>> assumption was that this is enough.  If a user wants to change the
>> default, he will most probably see the docstring and will have to
>> actively ignore the warning.
>
> I don't think this is enough. As you put it, it instills doubt without
> explaining anything. Why 35? Why "may not work"? What "may not work"?

Agreed.  I attach a patch with a more verbose docstring.

It is perhaps still not ideal - in particular, the warning is not
visible in the Customize interface - but I do not think this is a big
deal.  My line of thinking is that:

- if a user wants to change this setting, they will either look up the
  docstring and understand the limitation (btw, even the built-in way
  works for org-clock-history-length as high as 76 or so, provided you
  have a really high frame), or

- use Customize, which is potentially a trouble - but in that case,
  I would assume that the user fiddles with org-clock-history-length
  because they clock in many tasks, and then they will see that the list
  in the *Clock Task Select* buffer is too long anyway, and dial the
  setting down.

An alternative could be to change the hard-coded 35 in the code into yet
another variable, say, org-clock-history-max-length, and set it to 35.
Still, if a user wants to increase org-clock-history-length beyond 35
(or whatever this could be changed to), it is IMHO the /current/
behavior which is really confusing, and introducing a cap on the cap
would only make things worse.

WDYT?

--
Marcin Borkowski
http://mbork.pl

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Lift-the-artificial-limit-on-the-clock-history-lengt.patch --]
[-- Type: text/x-patch; size=1.41KiB, Size: 1443 bytes --]

From bd5c4b7f3afe6a906b91e7f0bd33e2842fcf8656 Mon Sep 17 00:00:00 2001
From: Marcin Borkowski <mbork@mbork.pl>
Date: Fri, 31 Aug 2018 07:53:42 +0200
Subject: [PATCH] Lift the artificial limit on the clock history length.

The default limit of 35 was hard-coded, and was especially annoying
when using an alternative way of selecting from history,
e.g. https://github.com/unhammer/org-mru-clock.
---
 lisp/org-clock.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 9819fcff2..71ed99f69 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -156,7 +156,10 @@ state to switch it to."
 	  (symbol :tag "Function")))
 
 (defcustom org-clock-history-length 5
-  "Number of clock tasks to remember in history."
+  "Number of clock tasks to remember in history.
+Clocking in using history works best if this is at most 35, in
+which case all digits and capital letters are used up by the
+*Clock Task Select* buffer."
   :group 'org-clock
   :type 'integer)
 
@@ -534,7 +537,7 @@ cannot be translated."
 
 (defun org-clock-history-push (&optional pos buffer)
   "Push a marker to the clock history."
-  (setq org-clock-history-length (max 1 (min 35 org-clock-history-length)))
+  (setq org-clock-history-length (max 1 org-clock-history-length))
   (let ((m (move-marker (make-marker)
 			(or pos (point)) (org-base-buffer
 					  (or buffer (current-buffer)))))
-- 
2.19.0

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Nicolas Goaziou]

Hello,

Marcin Borkowski <mbork@mbork.pl> writes:

> I attach a patch with a more verbose docstring.
>
> It is perhaps still not ideal - in particular, the warning is not
> visible in the Customize interface - but I do not think this is a big
> deal.  My line of thinking is that:
>
> - if a user wants to change this setting, they will either look up the
>   docstring and understand the limitation (btw, even the built-in way
>   works for org-clock-history-length as high as 76 or so, provided you
>   have a really high frame), or
>
> - use Customize, which is potentially a trouble - but in that case,
>   I would assume that the user fiddles with org-clock-history-length
>   because they clock in many tasks, and then they will see that the list
>   in the *Clock Task Select* buffer is too long anyway, and dial the
>   setting down.

It sounds good. I applied your patch. Thank you.

Regards,

-- 
Nicolas Goaziou

Re: How about lifting the limit of 35 tasks in org-clock-history?

[Marcin Borkowski]

On 2018-10-03, at 12:51, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> Marcin Borkowski <mbork@mbork.pl> writes:
>
>> I attach a patch with a more verbose docstring.
>>
>> It is perhaps still not ideal - in particular, the warning is not
>> visible in the Customize interface - but I do not think this is a big
>> deal.  My line of thinking is that:
>>
>> - if a user wants to change this setting, they will either look up the
>>   docstring and understand the limitation (btw, even the built-in way
>>   works for org-clock-history-length as high as 76 or so, provided you
>>   have a really high frame), or
>>
>> - use Customize, which is potentially a trouble - but in that case,
>>   I would assume that the user fiddles with org-clock-history-length
>>   because they clock in many tasks, and then they will see that the list
>>   in the *Clock Task Select* buffer is too long anyway, and dial the
>>   setting down.
>
> It sounds good. I applied your patch. Thank you.

Thanks, too!  (Also for your extremely fast reply, comparing to my
one;-).)

Best,

--
Marcin Borkowski
http://mbork.pl