GNU bug report logs - #74994
Improve Emacs iCalendar support

Previous Next

Full log

Message #47 received at 74994 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Richard Lawrence <rwl <at> recursewithless.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 74994 <at> debbugs.gnu.org
Subject: Re: bug#74994: Improve Emacs iCalendar support
Date: Mon, 20 Jan 2025 14:14:56 +0200

> From: Richard Lawrence <rwl <at> recursewithless.net>
> Cc: 74994 <at> debbugs.gnu.org
> Date: Sun, 19 Jan 2025 21:24:04 +0100
> 
> 1) Is it alright that this is a ground-up rewrite, rather than an
> evolution of icalendar.el? And how should I be thinking about the future
> of icalendar.el?
> 
> Having written this code, I can now understand and appreciate some of
> the code in icalendar.el better, but am still of the opinion that it is
> too closely tied to diary import/export to be as useful to other parts
> of Emacs (especially Org and Gnus) as it could be.

If you still think it's better to have a complete implementation, I
think you've answered your question, and I don't see anyone around who
knows this better and could challenge your opinions.

> I've been thinking it would make sense eventually to:
>   - rename icalendar.el to something like diary-icalendar.el
>   - rewrite the code there to use the new parser in icalendar-parser.el
>   - rename the user-facing functions
>     (icalendar-{import/export}-file and their variants)
>     to make clear that they are diary-specific
>   - delete(?)/deprecate the old parsing code

I'd prefer to do this without renaming the old file.  That is, rename
your new implementation to, say, icalendar2.el.  The rest of your plan
sounds good to me.

> 2) It would be nice if someone who has more experience writing parsers
> could give me some feedback about the overall design of the parser. In
> particular:
> 
>   - I've used macros (see icalendar-macs.el) to create a small "DSL" for
>     defining iCalendar types. These macros store parsing-related information for
>     each type as properties of the symbols which name them. There's a lot of
>     dynamic dispatch in the parser based on these type symbols' properties.
>     This adds some complexity but (I hope) makes the parser more "atomic"/
>     extensible. Does this seem like a reasonable approach in general?
>   - I ran into one issue that feels like a design flaw: the parser separates
>     "reading" (converting a string to an Elisp value) into a function
>     distinct from the parsing function which matches that string (see e.g.
>     ical:parse-property-value in icalendar-parser.el, which calls
>     ical:read-property-value). In simple cases this nicely factors out a pure
>     function from one which depends on a lot of global buffer state;
>     but in more complicated cases the "pure" reader function depends on
>     the match data and so isn't pure at all (see e.g. ical:read-dur-value).
>     Is there a better way to do this? (Not make the distinction? Pass
>     the match data explicitly? ...?)
>   - I haven't really thought about performance at all. My parser is
>     definitely slower/more memory intensive than icalendar.el's at the moment
>     (though hopefully this is mostly because it has more to offer). Are there
>     any things I should already be thinking about to make it more efficient?
> 
> 3) Likewise, part of my patch was icalendar-mode.el, and it would be
> useful to have some feedback about the major mode:
> 
>   - whether an icalendar-mode is even a useful thing to have, and what
>     could make it more useful
>   - whether the approach to syntax highlighting is reasonable (the
>     icalendar-define-* macros define matching functions for each
>     individual icalendar type, which are eventually piled
>     into a long list of matchers in the mode's value for font-lock-keywords)
>   - whether there's a better solution to the problem of needing to unfold
>     lines *before* a buffer containing iCalendar data is decoded
>     (is there anything like a hook that runs before decoding?)
>   - what still needs to be done to make the code in icalendar-mode.el
>     compliant with Emacs' major mode conventions (see the long list of
>     TODOs in the mode definition)

I added Stefan to the discussion, in the hope that he could give you
some useful advice about (some of) these aspects.

> > I can see already that your doc strings are not always according to
> > our conventions (the first line must be a single complete sentence),
> > and also the commit log messages need to be more detailed.  But these
> > are aspects best deferred until you have a version close to the final
> > one, now is too early for that.
> 
> OK, but that's good to keep in mind, thanks.
> 
> (Re: commit messages: should I be making commits in Emacs' ChangeLog
> format as I go along, and post a whole new patch series here after
> revisions? Or does it make more sense to squash everything at the end
> into one or a few commits, and only format the log messages at that
> point?)

We usually do the latter.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.