* Re: [elpa] master aa5513d 1/7: excorporate-diary: Always use diary-fancy-display [not found] ` <20190614211219.59B5721012@vcs0.savannah.gnu.org> @ 2019-06-15 10:32 ` Stefan Monnier 2019-06-15 13:13 ` Thomas Fitzsimmons 0 siblings, 1 reply; 4+ messages in thread From: Stefan Monnier @ 2019-06-15 10:32 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: emacs-devel > + (unless (eq diary-display-function 'diary-fancy-display) > + (warn (format > + (concat "Excorporate diary support needs diary-fancy-display" > + " but diary-display-function is currently %S; overriding") > + diary-display-function)) > + (customize-set-variable 'diary-display-function 'diary-fancy-display)) The warning is good, but I think that unilaterally overriding the user's choice is a bad idea. For example, it prevents using a tweaked version of `diary-fancy-display`. Stefan ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [elpa] master aa5513d 1/7: excorporate-diary: Always use diary-fancy-display 2019-06-15 10:32 ` [elpa] master aa5513d 1/7: excorporate-diary: Always use diary-fancy-display Stefan Monnier @ 2019-06-15 13:13 ` Thomas Fitzsimmons 2019-06-16 22:56 ` Stefan Monnier 0 siblings, 1 reply; 4+ messages in thread From: Thomas Fitzsimmons @ 2019-06-15 13:13 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Hi, Stefan Monnier <monnier@iro.umontreal.ca> writes: >> + (unless (eq diary-display-function 'diary-fancy-display) >> + (warn (format >> + (concat "Excorporate diary support needs diary-fancy-display" >> + " but diary-display-function is currently %S; overriding") >> + diary-display-function)) >> + (customize-set-variable 'diary-display-function 'diary-fancy-display)) > > The warning is good, but I think that unilaterally overriding the user's > choice is a bad idea. > > For example, it prevents using a tweaked version of `diary-fancy-display`. Thanks for reviewing. Yeah, I didn't like having to do that but the stock alternative, diary-simple-display, is broken. Specifically, even with diary-include-other-diary-files in diary-list-entries-hook, diary-simple-display doesn't show included entries. Maybe that's intended behavior for diary-simple-display, but it makes it seem like Excorporate hasn't done anything (since all its entries are included entries). I'm not sure how to disallow diary-simple-display but allow tweaked versions of diary-fancy-display. (Currently I'm also relying on diary-fancy-display-mode-hook.) I suppose I could just leave the warning and remove the customize-set-variable call. I'll think about it for the next release. Maybe I should also file a bug about diary-simple-display not showing included entries. Thomas ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [elpa] master aa5513d 1/7: excorporate-diary: Always use diary-fancy-display 2019-06-15 13:13 ` Thomas Fitzsimmons @ 2019-06-16 22:56 ` Stefan Monnier 2019-06-17 22:18 ` Thomas Fitzsimmons 0 siblings, 1 reply; 4+ messages in thread From: Stefan Monnier @ 2019-06-16 22:56 UTC (permalink / raw) To: emacs-devel > Thanks for reviewing. Yeah, I didn't like having to do that but the > stock alternative, diary-simple-display, is broken. FWIW, the default is already the fancy display: % grep diary-display-function **/*.el [...] lisp/calendar/diary-lib.el:(defcustom diary-display-function #'diary-fancy-display So if the user gets broken behavior it's presumably because of a specific choice in its config, I think it's OK to just explain the reason to the user without trying to impose a particular fix. Stefan ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [elpa] master aa5513d 1/7: excorporate-diary: Always use diary-fancy-display 2019-06-16 22:56 ` Stefan Monnier @ 2019-06-17 22:18 ` Thomas Fitzsimmons 0 siblings, 0 replies; 4+ messages in thread From: Thomas Fitzsimmons @ 2019-06-17 22:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Thanks for reviewing. Yeah, I didn't like having to do that but the >> stock alternative, diary-simple-display, is broken. > > FWIW, the default is already the fancy display: > > % grep diary-display-function **/*.el > [...] > lisp/calendar/diary-lib.el:(defcustom diary-display-function #'diary-fancy-display > > So if the user gets broken behavior it's presumably because of > a specific choice in its config, I think it's OK to just explain the > reason to the user without trying to impose a particular fix. Good point, pushed a change so that only the warning is issued. Thomas ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-17 22:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20190614211218.5886.59920@vcs0.savannah.gnu.org> [not found] ` <20190614211219.59B5721012@vcs0.savannah.gnu.org> 2019-06-15 10:32 ` [elpa] master aa5513d 1/7: excorporate-diary: Always use diary-fancy-display Stefan Monnier 2019-06-15 13:13 ` Thomas Fitzsimmons 2019-06-16 22:56 ` Stefan Monnier 2019-06-17 22:18 ` Thomas Fitzsimmons
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.