Ihor Radchenko <[email protected]> writes: > Hmm. I look at this differently. > We are fixing a number of bugs in org-clock-timestamp-change. > Those fixes revealed that some callers of org-clock-timestamp-change do > not call it according to the docstring, which worked in the past by > chance (and with strange workarounds further in the call chain).
I think you believe it's worse then it is. As long as we have enough calls to `prefix-numeric-value' sprinkled throughout the code then everything works as expected. Is that strictly correct? No, not at all. But it does work just fine. I am going to fix it but I see no reason to rush. Also as you'll see in my to be released thread about fixing these, this is not the only place we do this. > So, we need several fixes in a row. Either squashed or as separate > commits. > > Also, I do not see tiny commit as a problem. If a commit is fixing a > real problem, it is worth it, even if is it a one letter change. Guix (my training ground) cares a lot about the "one commit one change" stuff. However, that leads to a lot of tiny commits which does urk me. If this was truly the only place this issue occured, I would do a tiny commit. I agree that sometimes a one letter change is worth it. It's simply that this doesn't have to be a tiny commit if I put it in a little more effort. >> Well you've done it. Now I'm going through all the interactive >> declarations and fixing things. > > That was not my intention. Damn. I probably got an email tone issue. It's easier to keep things fun and light-hearted when everyone eats lunch together. I'll go ask chatgpt for advice :P >> I mostly want to know what you think of "Add match groups to >> `org-element-clock-line-re'" > > Match groups are ok. > If we want to be super-careful, we may announce the change in news or > rewrite it to have "no group" version as old variable and introduce a > new with groups, but I think the change you proposed has low chance to > break things and should be ok as is. My main concern is that I know people will start using this regex so if you don't like the 3 groups I picked (timestamp 1, timestamp 2, duration) now is the time to change them. Applied, as the following commits: e293a2692 lisp/org-clock.el: Rewrite 'org-clock-timestamps-change' b07138cb5 org-element: Add match groups to `org-element-clock-line-re' This message is being sent to update our issue tracker at https://tracker.orgmode.org
