GNU bug report logs -
#74994
Improve Emacs iCalendar support
Previous Next
Full log
View this message in rfc822 format
>> 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
I don't have an opinion on the file naming part, but I suggest you scrap
the "eventually" above, otherwise there's a strong risk it'll never
happen (either for lack of interest or because the API that doesn't
quite fit).
>> 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:
[ Note: I have not yet had time to look at the code. ]
>> - 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?
It sounds like a reasonable design, yes.
In `bindat.el` I used a similar approach except that each construct (I
guess in your case, that means each "type") is stored as a method (in
a generic function) instead of a property of a symbol. I'm not sure
it's the perfect solution, but it's nice that `C-h o` on the generic
function can then provide a documentation of each of the constructs.
Other options we use elsewhere is to use function names constructed from
a constant prefix plus the name of the construct, so instead of
(funcall (get 'foo 'bar) ...)
you might be able to macroexpand to something like
(,(intern (format "bar %s" 'foo)) ...)
so you get (for free) compile-time warnings when using a construct that
doesn't exist, and you avoid a `get` at runtime (IIRC, we use that
approach in `peg.el`).
>> - 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? ...?)
Is the separation useful to users (including internal users) of the
parser? This kind of problem doesn't directly ring a bell, so I don't
have a good suggestion to make. Maybe when I get time to read the
code... In the mean time, I can just mention that it reminds of the PEG
parsers generated by `peg.el` which use a limited form of backtracking:
they have a pure(ish) part that "explores" and accumulates "actions to
perform if we find a match" and only once we found a global parse do we
actually run the (side-effecting) actions which might build a parse tree.
>> - whether an icalendar-mode is even a useful thing to have, and what
>> could make it more useful
I don't think we need to be shy about major modes. If you don't use it,
it doesn't cost much (because it's not loaded), so count me as "+1".
>> - 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)
>> - 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)
[ Will see when I finally read the code. ]
>> - 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?)
[ Sorry, I don't understand this question. ]
Stefan
This bug report was last modified 99 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.