emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [O] More clocktable breakage


From: Nicolas Goaziou
Subject: Re: [O] More clocktable breakage
Date: Sun, 30 Apr 2017 09:21:47 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)

Hello,

Achim Gratz <address@hidden> writes:

> That's what I've been asking the whole time: when you changed that
> predicate, you've basically cut off some of its consumers.  It looks
> like bad factoring to me to then splitting the same predicate based on
> some argument.  I'd rather pull out the old sloppy matching into a new
> predicate for instance.

There are drawbacks in both choices. More on this below.

> I don't use planning, so no.  But it seems to me that the only way for
> org-element-context to deliver a 'timestamp is when pos is on a planning
> line (that may be wrong, I just don't see where else a 'timestamp would
> be returned).  In that case we've already left the cond, so testing for it
> doesn't do anything useful.  I'm also not really sure why the existing
> exceptions from the "true" timestamp (planning, property and clock-log)
> are any different than "dynamic block" would be (with the appropriate
> change of the doc string of course).y

I start to see where the confusion comes from. 

Sadly, what a "timestamp" is depends on what function is asking. AFAICT,
there are three categories of "timestamps".

Let's consider the following examples:

  (1)
  SCHEDULED: <2017-04-30 dim.>

  (2)
  CLOCK: [2017-04-30 dim. 08:10]--[2017-04-30 dim. 08:11] =>  0:01

  (3)
  : <2017-04-30 dim.>

  (4)
  :PROPERTIES:
  :DATE: <2017-04-30 dim.>
  :END:

  (5)
  # <2017-04-30 dim.>

  (6)
  #+BEGIN: clocktable :tstart "<2006-08-10 Thu 10:00>"

The first category is the "strict" one, which follows Org syntax
definition. None of the examples above belong to that category.
`org-element-context' should be the relative predicate, which means that
it probably shouldn't return a timestamp object on planning lines, as it
does at the moment.

The second category, which is a super-set of the previous one, is
"agenda" one. Historically, Org allowed to insert timestamps in property
drawers, e.g., as in (4), and use them in the agenda. So basically, this
category contains "every timestamp that would appear as a plain
timestamp in the agenda if it were active". `org-at-timestamp-p' is
currently the relative predicate for that category. Note that the
category also contains (2) if inactive timestamps are meant to be
displayed in the agenda.

The last category, a super-set of the "agenda" one, is the "convenience"
category. Basically, it contains every occurrence of what looks like
a timestamp, but isn't necessarily one. Obviously, every example given
above fits in there, as in every case, you can ignore that Org has
a context-dependant grammar and pretend you are on a timestamp. There is
no predicate relative to that category, but `org-at-timestamp-p'
docstring suggests to use (org-in-regexp org-ts-regexp).

So, about the predicates, I guess we could move the second one into
"org-agenda.el" and rename it `org-agenda-at-timestamp-p'. However,
using `org-at-timestamp-p' for the last category seems a bit
far-stretched to me, since it doesn't always match timestamps. In
particular, (3) and (5) sound counter-intuitive to me and I wouldn't
like it from a developer standpoint. `org-at-timestamp' is also not
really needed for the first category, since there is already
`org-element-context'.

Yet, `org-at-timestamp-p' is something users are going to look after.
Different names may also introduce confusion (remember `org-at-regexp-p'
and `org-in-regexp-p'?). So, even if it looks like bad factoring to you,
a single predicate, or even two if we set "agenda" apart, with
a docstring explaining the different categories and how to select them
may also be a good natural choice. Hence my suggestion.

WDYT? Do you have any other concrete proposal?

Regards,

-- 
Nicolas Goaziou



reply via email to

[Prev in Thread] Current Thread [Next in Thread]