org-super-agenda icon indicating copy to clipboard operation
org-super-agenda copied to clipboard

org-super-agenda-groups needs a custom-type definition

Open TinaRussell opened this issue 5 years ago • 11 comments

Right now, org-super-agenda-groups is defined like this:

(defcustom org-super-agenda-groups nil
  "List of groups to apply to agenda views when `org-super-agenda-mode' is on.
See readme for information."
  :type 'list)

When you edit the option in customize, it looks like this:

org-super-agenda-groups: nil
    State : STANDARD. (mismatch)

…and you can’t define the list using any customization widgets, you have to write bare Lisp syntax. I’m sure this is okay for you Emacs Lisp wizards, but to a n00b like me, it’s intimidating!

Since org-super-agenda-groups is a complicated (and powerful) option with its own syntax, a proper :type definition would be a godsend! Here’s a good example of a thorough definition for a complex option with specialized syntax, from org-capture.el:

(defcustom org-capture-templates nil
  ;; very very long docstring snipped here —Tina
  :group 'org-capture
  :version "24.1"
  :set (lambda (s v) (set s (org-capture-upgrade-templates v)))
  :type
  (let ((file-variants '(choice :tag "Filename       "
				(file :tag "Literal")
				(function :tag "Function")
				(variable :tag "Variable")
				(sexp :tag "Form"))))
  `(repeat
    (choice :value ("" "" entry (file "~/org/notes.org") "")
	    (list :tag "Multikey description"
		  (string :tag "Keys       ")
		  (string :tag "Description"))
	    (list :tag "Template entry"
		  (string :tag "Keys           ")
		  (string :tag "Description    ")
		  (choice :tag "Capture Type   " :value entry
			  (const :tag "Org entry" entry)
			  (const :tag "Plain list item" item)
			  (const :tag "Checkbox item" checkitem)
			  (const :tag "Plain text" plain)
			  (const :tag "Table line" table-line))
		  (choice :tag "Target location"
			  (list :tag "File"
				(const :format "" file)
				,file-variants)
			  (list :tag "ID"
				(const :format "" id)
				(string :tag "  ID"))
			  (list :tag "File & Headline"
				(const :format "" file+headline)
				,file-variants
				(string :tag "  Headline"))
			  (list :tag "File & Outline path"
				(const :format "" file+olp)
				,file-variants
				(repeat :tag "Outline path" :inline t
					(string :tag "Headline")))
			  (list :tag "File & Regexp"
				(const :format "" file+regexp)
				,file-variants
				(regexp :tag "  Regexp"))
			  (list :tag "File [ & Outline path ] & Date tree"
				(const :format "" file+olp+datetree)
				,file-variants
				(option (repeat :tag "Outline path" :inline t
						(string :tag "Headline"))))
			  (list :tag "File & function"
				(const :format "" file+function)
				,file-variants
				(sexp :tag "  Function"))
			  (list :tag "Current clocking task"
				(const :format "" clock))
			  (list :tag "Function"
				(const :format "" function)
				(sexp :tag "  Function")))
		  (choice :tag "Template       "
			  (string)
			  (list :tag "File"
				(const :format "" file)
				(file :tag "Template file"))
			  (list :tag "Function"
				(const :format "" function)
				(function :tag "Template function")))
		  (plist :inline t
			 ;; Give the most common options as checkboxes
			 :options (((const :format "%v " :prepend) (const t))
				   ((const :format "%v " :immediate-finish) (const t))
				   ((const :format "%v " :jump-to-captured) (const t))
				   ((const :format "%v " :empty-lines) (const 1))
				   ((const :format "%v " :empty-lines-before) (const 1))
				   ((const :format "%v " :empty-lines-after) (const 1))
				   ((const :format "%v " :clock-in) (const t))
				   ((const :format "%v " :clock-keep) (const t))
				   ((const :format "%v " :clock-resume) (const t))
				   ((const :format "%v " :time-prompt) (const t))
				   ((const :format "%v " :tree-type) (const week))
				   ((const :format "%v " :unnarrowed) (const t))
				   ((const :format "%v " :table-line-pos) (string))
				   ((const :format "%v " :kill-buffer) (const t)))))))))

As the org-super-agenda documentation says, “At first you might feel bewildered by all the options.” The advantage of having a thorough :type definition for customize is that users can tackle one decision at a time, with the different choices clearly displayed at each juncture, without having to go back and forth between code and documentation or keep parentheses balanced or whatever, and without having to have the entire idea fully formed in one’s head from the beginning. The documentation for :type is here: elisp: Customization Types Thanks for all your hard work! :smile:

TinaRussell avatar Feb 03 '19 19:02 TinaRussell

Please see https://github.com/alphapapa/org-super-agenda/blob/master/notes.org#b-define-customization-types-for-group-selectors. It would be a nice feature to have, and maybe I'll do it someday. In the meantime, patches welcome.

alphapapa avatar Feb 03 '19 21:02 alphapapa

I'm building a type analyzer into Elsa to understand the defcustom type annotations so that would also help when people configure by hand. But it can get quite insane.

Fuco1 avatar Mar 07 '19 21:03 Fuco1

So I’ve been hacking away at this for a few days now. My progress so far is here: https://github.com/TinaRussell/org-super-agenda/tree/custom-types I’m trying to keep within the style of the existing code, adding a keyword to the group-defining macros so that each group can have its own custom-type definition.

I guess the biggest hiccups I’ve so far run into are:

  1. It looks weird; the `plist' composite type displays every single possible key, whether or not you’ve even checked its box (i.e. whether or not you actually use that key). There doesn’t seem to be an easy way to have them start off collapsed, like when customizing the attributes of a face. (This could probably be done with a custom widget, but how to do that efficiently is beyond me; I’ve never worked with the widget library much before.)
  2. Where/how is order-multi implemented? It’s the only thing in the org-super-agenda-groups example (the one in the README, etc.) that my big ol’ custom type definition can’t understand (it displays that part as a bare sexp). I can’t find order-multi implemented anywhere in the code, though, so I’m not sure how to make it work (or if it’s something left over in the example from an old version, etc).

TinaRussell avatar Nov 25 '19 10:11 TinaRussell

I appreciate your work on this, and the code in your branch looks pretty good. I like the way you added to the macros.

However, as you noted, the UI doesn't turn out great with the existing customization widgets. I noticed similar issues with some of my other packages' customization options, where it seemed difficult to read the boxes and labels, especially if any line wrapping was involved. I feel like it's actually much harder to read the structure of the sexps in the customization UI than as plain lisp code. And as you noted, displaying every possible key is a significant drawback; even though some users might find the GUI customization helpful, displaying all keys would be bewildering and cumbersome. A new widget might help, maybe one that displayed nested structures more clearly, but I also haven't used that library before, and implementing a new widget for this feels a bit out-of-scope for this project.

So I'm probably going to postpone this feature idea indefinitely. Some things are just better done as lisp code. :)

BTW, order-multi is in the file, just search for it and you'll find the associated code.

alphapapa avatar Nov 27 '19 21:11 alphapapa

Uh huh. I should note I did make a little bit of progress on a different widget-related front: I realized, because most of the specifiers take arguments in the form :key VALUE or :key (VALUE VALUE …), I needed an easy way to have a list that’s either one item by itself or multiple items in a list. Surprisingly, I was able to make such a thing without much code: https://emacs.stackexchange.com/questions/54019/ (look for my own answer, which I’ll be allowed to accept… tomorrow, weird). I’m hoping the other UI problem (the loads of unused fields) is something I can solve, too, maybe with some hints from custom-face-edit.

Ohhhh, I finally figured out how order-multi works… I see, the association is set in org-super-agenda-group-transformers. That makes sense now. Thanks!

TinaRussell avatar Nov 28 '19 01:11 TinaRussell

Cool, if there were a way to simplify the widget UI, it might be worth considering. Thanks.

alphapapa avatar Dec 21 '19 20:12 alphapapa

So, I’m pretty close to making this all work. The last piece of functionality I need to implement is the group transformers, regarding which I have an API question. I know that :order-multi takes an integer as an argument, followed by any number of groups; if more group transformers are implemented, will they also take one argument followed by any number of groups? (Like how the auto-grouping selectors each take one argument, even if it’s just t.) Or, is that up to how the transformer function is defined?

TinaRussell avatar Mar 05 '20 05:03 TinaRussell

Probably, but I haven't given that any thought since there haven't been any more transformers.

alphapapa avatar Mar 05 '20 12:03 alphapapa

Okay, that’s fine. The way I’m thinking is, there are two options:

  1. All group transformers take one argument, even if just t, followed any number of groups. A plist, org-super-agenda-group-transformer-arg-types, will associate group transformers with custom-type specifications (for instance, integer for :order-multi). If a group transformer has no entry in the plist, its argument type is assumed to be (const t).

  2. (more complicated, more versatile) All group transformers can potentially take any number of arguments, including none at all, followed by any number of groups. A plist, org-super-agenda-group-transformer-arg-types, will associate group transformers with either a custom-type specification (for one leading argument) or a list of custom-type specifications (for multiple leading arguments). If a group transformer has no entry in the plist, it is assumed to take no arguments other than groups.

TinaRussell avatar Mar 05 '20 23:03 TinaRussell

I don't want to commit to doing it one way or the other now. I'd recommend just doing what is necessary given the current state of the code.

I am interested to see what you've come up with, but I can't promise to merge the code in advance. I don't want to give you a false impression. I generally feel like the benefit is slight and the cost in code too heavy. In general, some parts of Org require editing Lisp code or lists, and I'm okay with that here too. It might be best if you publish your code on your own repo as an add-on, and I'd be happy to link to it in the documentation for users who are interested.

alphapapa avatar Mar 06 '20 17:03 alphapapa

I'm currently rewriting my init.el using the new Emacs-29 macro setopt instead of setq, to catch bugs due to wrong types. I found that org-super-agenda-groups raises a type warning even if configured according to the examples (just with setopt), e.g.

(setopt org-super-agenda-groups
        '((:name "Today" :time-grid t :todo "TODAY")
          (:name "Important" :tag "bills" :priority "A")))

which raises

⛔ Warning (emacs): Value ‘((:name "Today" :time-grid t :todo "TODAY") (:name "Important" :tag "bills" :priority "A"))’
does not match type list.

In this particular case there is no danger in just using setq to avoid the warning, but since the variable was declared with defcustom, I would expect setopt to be usable and not give warnings when used correctly.

I tested this on the Emacs master branch for Emacs 30 but I think this must be reproducable in Emacs 29.

I'm not sure if there's a simple catch-all type that can replace :type list to avoid this warning, or if this requires actually solving this issue here and providing a specific type-definition for enabling customization widgets.

meliache avatar Nov 03 '23 01:11 meliache