-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merging ts.el into Emacs core #29
Comments
Various benchmarks from earlier in |
Adam Porter ***@***.***> writes:
Various benchmarks from earlier in `ts`'s development may be seen in the notes file: https://github.com/alphapapa/ts.el/blob/master/notes.org
I looked through your benchmarks and I cannot see how they justify a new
time representation:
1. https://github.com/alphapapa/ts.el/blob/master/notes.org#with-filling
demonstrates list vs vector access. This will not be considered
significant.
2. https://github.com/alphapapa/ts.el/blob/master/notes.org#accessor-dispatch-vs-string-to-number-format-time-string
is not fair. If you need to get year,
(string-to-number (format-time-string "%Y" unix)) is very awkward way
to do it. You should better use
(decoded-time-year (decode-time (current-time)))
All other benchmarks does not seem relevant to comparison with the
existing Emacs time APIs.
That said, *new* API functions introduced in ts.el might be of interest
for upstream.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
|
Note, of course, that
At this point, I would expect that the appeal would be the unified API for accessing parts of and manipulating timestamps. |
Adam Porter ***@***.***> writes:
Note, of course, that `decoded-time-` accessors did not exist in Emacs prior to `ts`. Those were added after `ts` had been released and used in the wild for some time.
Sure, but it does not matter if you want to merge ts.el upstream.
We need to compare with Emacs master.
> That said, *new* API functions introduced in ts.el might be of interest for upstream.
At this point, I would expect that the appeal would be the unified API for accessing parts of and manipulating timestamps.
I do not see how unified API would justify ts struct (there are already
too many time representations in Emacs; adding yet another one will be
questioned). It may be necessary to work with `decode-time'-returned
struct instead.
Further, I suggest providing some kind of summary comparing the
existing time API and what you change/add in ts.el.
This will help to see which aspects need to be improved and show why a
new set of functions is needed.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
|
I did not mean to propose upstreaming the struct, just the API.
Of course, it would be unlikely to be accepted otherwise. However, this is one of the most time-consuming aspects of such a proposal, the "selling" and convincing via laborious examples, ones which seem obviously beneficial to those who need them, but will be repeatedly questioned by those who don't write related code and can't imagine the benefit. Do you agree that having a unified API for working with time values, like the one in As well, if a proposal were accepted upstream, would you be willing to help with writing the code and/or documentation? Thanks. |
Adam Porter ***@***.***> writes:
Do you agree that having a unified API for working with time values,
like the one in `ts`, is useful, and would be a good thing to have in
Emacs? Or would I need to argue for this by myself?
Org mode uses ad-hoc time API to work with time shifts and time in
general. In particular, `org-current-effective-time',
`org-fix-decoded-time', `org-closest-date',
`org-table-time-seconds-to-string', `org-time=/</>/<>/<=/>=', and
`org-read-date'.
This API is invented out of necessity. Org more would use native API if
Emacs had one.
As well, if a proposal were accepted upstream, would you be willing to
help with writing the code and/or documentation?
Sure, I can help. Especially to implement/document equivalents of
functions and commands used in Org mode.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
|
Here's a very simple initial example of how the (let ((decoded (decode-time argh-time)))
(cl-incf (decoded-time-day decoded) 365)
(format-time-string "%c" (encode-time decoded)))
;; "Fri Feb 21 20:28:47 2025"
(ts-format "%c" (ts-adjust 'day 365 (ts-now)))
;; "Fri Feb 21 20:34:18 2025" Also of note is the differing values: while I don't claim that In fact, there seems to be some kind of weird overflow happening, because as I write this message and experiment with code that modifies the seconds slot instead of the day slot, I notice that the time in the result doesn't change, even though it's being evaluated a few minutes later: (let ((decoded (decode-time argh-time)))
(cl-incf (decoded-time-second decoded) (* 60 60 24 365))
(format-time-string "%c" (encode-time decoded)))
;; "Fri Feb 21 20:28:47 2025" So to get a more accurate result requires working on the time value in terms of seconds and internal time values, like: (let ((time (current-time)))
(setf time (time-add (seconds-to-time (* 60 60 24 365)) time))
(format-time-string "%c" time))
;; "Fri Feb 21 20:39:52 2025" Next to which (ts-format "%c" (ts-adjust 'day 365 (ts-now)))
;; "Fri Feb 21 20:34:18 2025" So the idea of refactoring However, another matter to keep in mind is that, since What do you think? Thanks. |
Adam Porter ***@***.***> writes:
Here's a very simple initial example of how the `ts` API is easier to use to manipulate a timestamp than the current Emacs native ones:
```el
(let ((decoded (decode-time argh-time)))
(cl-incf (decoded-time-day decoded) 365)
(format-time-string "%c" (encode-time decoded)))
;; "Fri Feb 21 20:28:47 2025"
(ts-format "%c" (ts-adjust 'day 365 (ts-now)))
;; "Fri Feb 21 20:34:18 2025"
```
I would not call it "unified API". `ts-adjust' is simply missing from
Emacs built-in libraries. Proposing a set of new time-related functions
to Emacs upstream is easier than proposing a new API, I think.
Also of note is the differing values: while I don't claim that `ts`'s
results are always necessarily "correct" for this kind of thing (as
taking into account leap years, leap seconds, and etc. is non-trivial,
and `ts` doesn't even try to do this itself),
I think it _is_ trivial. But you need to use things like `time-add' that
make use of C time libs that are aware about IANA timezone DB.
it's notable that the
result of changing the decoded time day and then re-encoding it is off
by about 6 minutes.
Which is likely because you assigned argh-time value 6 minutes before
running the above code. If you instead use (current-time), you will get
consistent results (as expected). At least, I do.
However, another matter to keep in mind is that, since `ts` uses Unix
timestamps internally, it helps to deal with time-zone issues, as Unix
timestamps are in UTC by definition; whereas Emacs's internal time
values don't have a defined time zone, so users must either ensure
they are converted to UTC or carry the associated time zone
separately.
That's good point. Although the difference is not because `ts' uses Unix
timestamp, but because `ts' explicitly stores time zone name. In
contrast, `decode-time' only provides UTC offset and does not retain the
time zone info. UTC offset alone is not enough to account for
timezone-specific rules about month lengths and daylight saving time
transitions.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
|
Ihor Radchenko ***@***.***> writes:
> However, another matter to keep in mind is that, since `ts` uses Unix
> timestamps internally, it helps to deal with time-zone issues, as Unix
> timestamps are in UTC by definition; whereas Emacs's internal time
> values don't have a defined time zone, so users must either ensure
> they are converted to UTC or carry the associated time zone
> separately.
That's good point. Although the difference is not because `ts' uses Unix
timestamp, but because `ts' explicitly stores time zone name. In
contrast, `decode-time' only provides UTC offset and does not retain the
time zone info. UTC offset alone is not enough to account for
timezone-specific rules about month lengths and daylight saving time
transitions.
I was not accurate here. `encode-time' actually understands non-offset
time zones like "Asia/Singapore". It is just that `decode-time' only
returns the UTC offset. And tz-abbrv slot is not used in practice (so,
it is hard to justify adding it).
To move things forward, let me provide a tentative list of functions
that might be added to Emacs upstream:
ts-parse-fill, ts-apply, ts-human-duration, ts-human-format-duration,
ts-adjust, ts-inc, ts-dec, ts-adjustf, ts-incf, ts-decf, ts-in, ts<=,
ts>, ts>=.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
|
Let me know if I can help anything to move things forward. |
I appreciate that. The next few weeks will be especially busy for me, and I won't have much time for Emacs-related things. Anyway, your tentative list seems good to me. I suppose a next step might be to propose such an API to emacs-devel--that is, without the internal things that |
It has been a few months. Just bumping the thread :) |
Thanks for the ping, Ihor. Haven't had much time lately for Emacs things. Maybe I can find some soon. |
Following up alphapapa/org-ql#409
@alphapapa listed the following reasons why ts.el may be beneficial compared to the existing time API:
The text was updated successfully, but these errors were encountered: