Re: [Patch] apps-menu

[clemens fischer <ino-news spotteswoode dnsalias org>]

On Fri-2010/08/20-15:29 Matthew Love wrote:

> Here is another patch for apps-menu.jl.

Just a style issue:

  diff --git a/lisp/sawfish/wm/ext/apps-menu.jl b/lisp/sawfish/wm/ext/apps-menu.jl
  index 4aeaec0..6fead3c 100644
  --- a/lisp/sawfish/wm/ext/apps-menu.jl
  +++ b/lisp/sawfish/wm/ext/apps-menu.jl
  @@ -158,23 +158,26 @@ set this to non-nil.")
          (substring string 0 (match-start))
         string))

  -  ;; This is wrong.  Read the desktop entry spec to see how it should
  -  ;; be done.  It's complicated.
  +  (defmacro simplify-mlang (mlang mlevel)
  +    `(and
  +      ,(if (or (= 0 mlevel) (not mlevel))
  +          `(or (string-looking-at "([a-z]*)(_?)([A-Z]*?)(@)([A-Z]*[a-z]*)?" ,mlang)
  +               (string-looking-at "([a-z]*)(_..)|([a-z]*)?" ,mlang)
  +               (string-looking-at "([a-z]*)?" ,mlang))
  +        (if (= 1 mlevel)
  +            `(string-looking-at "([a-z]*)(_?)([A-Z]*?)(@)([A-Z]*[a-z]*)?" ,mlang)
  +          (if (= 2 mlevel)
  +              `(string-looking-at "([a-z]*)(_..)|([a-z]*)?" ,mlang)
  +            (if (= 3 mlevel)
  +                `(string-looking-at "([a-z]*)?" ,mlang)))))
  +      (expand-last-match "\&")))

There are such things as "cond" and "case" in lisp, and in rep, too
(untested):

  (defmacro simplify-mlang (mlang mlevel)
    `(and
      ,(if (or (= 0 mlevel) (not mlevel))
          `(or (string-looking-at "([a-z]*)(_?)([A-Z]*?)(@)([A-Z]*[a-z]*)?" ,mlang)
               (string-looking-at "([a-z]*)(_..)|([a-z]*)?" ,mlang)
               (string-looking-at "([a-z]*)?" ,mlang))
        (case mlevel
            (1 `(string-looking-at "([a-z]*)(_?)([A-Z]*?)(@)([A-Z]*[a-z]*)?" ,mlang))
            (2 `(string-looking-at "([a-z]*)(_..)|([a-z]*)?" ,mlang))
            (3 `(string-looking-at "([a-z]*)?" ,mlang))))
      (expand-last-match "\&")))

Doesn't that read much better?  The nested "if"s look ugly to me.  As do
the duplicated string literals.  I don't know what they are supposed to
mean, and this will be true for other newbies.  Why not "(let ...)"
bind them up front and give them meaningful names?


clemens