ch11ng / exwm

Emacs X Window Manager
2.85k stars 134 forks source link

Add sensible types to defcustoms. Most notably use `:key-sequence` 'type' to simulation keys. #612

Open emacksnotes opened 5 years ago

emacksnotes commented 5 years ago

Add sensible types to defcustoms. Most notably use :key-sequence to configure simulation keys.

Hint: You need some variation of this.

(defcustom configure-simulation-keys nil
  "Configure simulated keys."
  :type '(choice
      (const :tag "None" nil)
      (alist :tag "Values"
         :key-type (key-sequence :tag "Orginal Key")
         :value-type (key-sequence :tag "Simulated Key"))))

The advantage of :key-sequence in the defcustom widget is this: In the key sequence field, I can use C-q input the keys via the keyboard. Emacs will take care of translating the key presses to it's internal vector format.

exwm-input-simulation-keys already does it. exwm-manage-configurations /does not/.

(If you have a non-FSF-assigned branch, I can cook up a proper patch.)

ch11ng commented 5 years ago

I hoped the same when implementing exwm-manage-configurations, but found no way to specify different type for each configuration in a same plist. Splitting the configurations would be sub-optimal as the user would have to repeat the matching criteria.

emacksnotes commented 5 years ago

Add sensible types to defcustoms. Most notably use :key-sequence 'type' to simulation keys in exwm-manage-configurations

I hoped the same when implementing exwm-manage-configurations, but found no way to specify different type for each configuration in a same plist. Splitting the configurations would be sub-optimal as the user would have to repeat the matching criteria.

How about something like this:

(defcustom some-plist
  `(:width 0
       :height 0
       :simulation-keys
       (([?\C-b] . [left])
        ([?\C-f] . [right])))
  "Some plist."
  :type `(plist :options
        (((const :tag "Width" :width) (integer :tag "Width"))
         ((const :tag "Height" :height) (integer :tag "Height"))
         ((const :tag "Simulation Keys" :simulation-keys)
          (alist :key-type (key-sequence :tag "Original" [ignore])
             :value-type (key-sequence :tag "Simulated" [ignore]))))))

In regard to the matcher, in addition to allowing the value-type to be a lisp form (to be eval-lled) you may want to allow the value-type to be a regexp. This regexp can first match the class-name /or/ (?) title in that order. (You know what is best) Allowing a string value, be easy on users who want to congiure exwm but aren't comfortable with elisp.

ch11ng commented 5 years ago

Thanks for the info! I've updated the definition of exwm-manage-configurations accordingly.

In regard to the matcher, in addition to allowing the value-type to be a lisp form (to be eval-lled) you may want to allow the value-type to be a regexp. This regexp can first match the class-name /or/ (?) title in that order. (You know what is best) Allowing a string value, be easy on users who want to congiure exwm but aren't comfortable with elisp.

It's hard to guess what users want the regexp to match against.

emacksnotes commented 5 years ago

Thanks for the info! I've updated the definition of exwm-manage-configurations accordingly.

I am happy with this change.

In regard to the matcher, in addition to allowing the value-type to be a lisp form (to be eval-lled) you may want to allow the value-type to be a regexp. This regexp can first match the class-name /or/ (?) title in that order. (You know what is best) Allowing a string value, be easy on users who want to congiure exwm but aren't comfortable with elisp.

It's hard to guess what users want the regexp to match against.

You don't /guess/ what the field the regexp has to match against. You /declare/ that if it is a string it will be matched against exwm-class-name. If I want to match against some other field or have very complex rules, I can always type out my sexp. To reiterate, my suggestion was /not/ to remove the sexp as a matcher, but to retain it while /also/ allowing for a string (as a regexp) as a matcher. IME, most people want to match against class name. You are in a better position to assess what is the /most commonly used field/ that users wish to match against.

My suggestion amounts to this:

In exwm-manage-configurations, you allow the :key-type to be a string / regexp

'(alist :key-type (choice
               (regexp :tag "Regexp to match against `exwm-class-name':" ".*")
               (sexp :tag "Matching criterion" nil))
                :value-type (...))

and in exwm-manage--get-configurations, instead of testing like this

(when (eval (car i) t)
          (cl-return-from exwm-manage--get-configurations (cdr i)))

you test like this

(when (if (stringp (car i))
      (string-match-p (car i) exwm-class-name)
    (eval (car i) t))    
  (cl-return-from exwm-manage--get-configurations (cdr i)))
ch11ng commented 5 years ago

You don't /guess/ what the field the regexp has to match against. You /declare/ that if it is a string it will be matched against exwm-class-name.

I'm not sure if this is the most frequently used property. Others may want exwm-instance-name or exwm-title instead. I personally use some more complicated matching criteria.

emacksnotes commented 5 years ago

You don't /guess/ what the field the regexp has to match against. You /declare/ that if it is a string it will be matched against exwm-class-name.

I'm not sure if this is the most frequently used property. Others may want exwm-instance-name or exwm-title instead. I personally use some more complicated matching criteria.

I am not talking about technicalities. I am talking about user-friendly configuration.

As a new user, I was confused about what could possibly the matcher be. This is because the docstring of exwm-manage-configurations in regard to what the matcher could be.

So ...

Revise docstring of exwm-manage-configurations, and particularly mention the fields one can match against --exwm-title exwm-class-name , exwm-instance-name etc

... and provide a sample example, say for 'Firefox'.

FYI, the docstring reads as below

For each X window managed for the first time, matching criteria (sexps) are evaluated sequentially and the first configuration with a non-nil matching criterion would be applied.

and it is very vague what one could match against.

Refine key-type field of exwm-manage-configurations

from

(sexp :tag "Matching criterion" nil)

to


      (radio
       (cons :tag "Window Title Matches" (const  :tag "" exwm-title)
         (regexp :tag "Regex" ))
       (cons :tag "Window Class Matches" (const  :tag "" exwm-class-name)
         (regexp :tag "Regex" ))
       (cons :tag "Window Instance Matches" (const  :tag "" exwm-instance-name)
         (regexp :tag "Regex" ))
       (sexp :tag "Matching criterion" nil))

With the above changes, the customization buffer would look like as below

Use radio button for keys in exwm-manage-configurations exwmmanageconfig

and modify exwm-manage--get-configurations to reflect the above changes.

The advantage of my suggestion is that the user can just go ahead with configuring exwm without having to spend another 30 mins to an hour figuring what his options are.

Let me reiterate, I am not suggesting that sexp matcher be removed. A power-user like you still has an option to go with a custom sexp type matcher.

ch11ng commented 5 years ago

Thanks for the elaborate explanation but sorry I'm still not convinced. The complexity is not necessary reduced considering the use of Elisp-flavor regular expressions, which I'm afraid most users have no clue how to compose them correctly. The problem with sexps is that users are not familiar with Elisp builtin functions. Were they aware of functions like string-prefix-p and string-suffix-p, they would probably find it much easier.

This is because the docstring of exwm-manage-configurations in regard to what the matcher could be.

Literally everything, and exwm-title, exwm-class-name are simply a few public interfaces exposed to users. Say you want to stick with more stable settings in weekdays and try radical ones on weekends, you are matching against date/time (just a silly example).

emacksnotes commented 5 years ago

Were they aware of functions like string-prefix-p and string-suffix-p, they would probably find it much easier.

IIUC, you are arguing for dumbing down things.

So,

Refine key-type field of exwm-manage-configurations

        (choice :tag "Matcher"
         (list :tag "Simple Matcher" (choice (const :tag "Starts with" string-prefix-p)
                             (const :tag "Ends with" string-prefix-p)
                             (const :tag "Contains" string-match-p))
               (regexp :tag "Pattern")
               (choice
            (const  :tag "Title" exwm-title)
            (const  :tag "Class" exwm-class-name)
            (const  :tag "Instance" exwm-instance-name))
               (boolean :tag "Ignore case" nil))
         (sexp :tag "Matching Predicate"))

which looks like

Screenshot from 2019-08-27 22-30-45

and you test like this

    (dolist (i exwm-manage-configurations)
      (save-current-buffer
        (when (ignore-errors (eval (car i) t))
          (cl-return-from exwm-manage--get-configurations (cdr i)))))

Note the ignore-errors above. This not only "protects" exwm against user errors, but also obviates the need for testing if the title, class etc are indeed non-null (i.e.,Tests like (and (stringp exwm-class-name) (string-match-p "Firefox" exwm-class-name)) simply becomes (string-match-p "Firefox" exwm-class-name))

The complexity is not necessary reduced considering the use of Elisp-flavor regular expressions,

:type regexp in defcustom is neither a rx-style regexp nor a read-able string. It is just a plain regexp that one would enter in a minibuffer for C-u C-s etc.

Here is an already existing option that uses regexp type.

(defcustom server-temp-file-regexp "\\`/tmp/Re\\|/draft\\'"
  "Regexp matching names of temporary files.
These are deleted and reused after each edit by the programs that
invoke the Emacs server."
  :type 'regexp)

Literally everything, and exwm-title, exwm-class-name are simply a few public interfaces exposed to users.

My point was that docstring should mention and link to the common window-specific variables that one usually matches against. If you fear that an explicit mention of these variables, may be mis-taken as the only fields that can be in the matcher sexp, you may want to qualify that it is not so (in much the same way that you have qualified your reply here).

ch11ng commented 5 years ago

Refine key-type field of exwm-manage-configurations [...] which looks like [...]

Still way too complex and sounds suboptimal to me. I'd rather stick with a single sexp option. Let's see if someone else would come up with such request.

and you test like this [...] Note the ignore-errors above.

It'd be really helpful. I'll take this advice.

:type regexp in defcustom is neither a rx-style regexp nor a read-able string. It is just a plain regexp that one would enter in a minibuffer for C-u C-s etc.

I mean the rules here are quite different from what users are already familiar with (BRE/ERE/PCRE, if they are).

My point was that docstring should mention and link to the common window-specific variables that one usually matches against.

I'll add that.