Re: check-lisp-parenthesis

[pjb@informatimago.com (Pascal J. Bourguignon)]

Cecil Westerhof <Cecil@decebal.nl> writes:

> I started with a function to check lisp parenthesis.

What's wrong with check-parens?

Your function does more than just checking parentheses.  It reformats
the code.  You could call it beautify-parens.


> Before a '(' I accept a space, a tab, a newline, a '(' and a ';'. When
> there is another character the user is asked if for the '(' a space has
> to be inserted.
>
> After a ')' I accept a space, a tab, a newline and a ')'. When there is
> another character the user is asked if after the ')' a space has to be
> appended.
>
> So far so good.
>
> But I also want to ask to remove white-space gaps. For example:
>               (message message)
>             ))))
> should be changed -when the user wants it- to:
>               (message message)))))
>
> The gap is found, but when I say that I want to delete the gab, this is
> not done. What am I doing wrong?
>
> The code:
>     (defun check-lisp-parenthesis ()
>       (interactive)
>       (if (not (interactive-p))
>           (message "check-lisp-parenthesis can only be called interactive")

There's no reason to be so restrictive!  

The right way to use interactive-p is:

   (when (if (interactive-p)
           (y-or-n-p "Insert a space?")
           t)
       (insert " "))


>         (save-excursion
>           (let ((found-gap-left     0)
>                 (found-gap-right    0)
>                 (found-left         0)
>                 (found-right        0)
>                 (inserted-gap-left  0)
>                 (inserted-gap-right 0)
>                 (inserted-left      0)
>                 (inserted-right     0)
>                 (message            ""))
>           (goto-char (point-min))
>           (while (re-search-forward "[^ \t\n(;](" nil t)
>             (setq found-left (1+ found-left))
>             (goto-char (forward-point -1))

I would rather use:
              (goto-char (1+ (match-beginning 0) ))

>             (when (y-or-n-p "Insert a space? ")
>               (insert " ")
>               (setq inserted-left (1+ inserted-left))))
>           (setq message (format "%sfound-left: %s, inserted-left: %s\n"
>                                 message
>                                 found-left  inserted-left))
>           (goto-char (point-min))
>           (while (re-search-forward ")[^ \t\n)]" nil t)
>             (setq found-right (1+ found-right))
>             (goto-char (forward-point -1))
>             (when (y-or-n-p "Append a space? ")
>               (insert " ")
>               (setq inserted-right (1+ inserted-right))))
>           (setq message (format "%sfound-right: %s, inserted-right: %s\n"
>                                 message
>                                 found-left  inserted-left))
>           (goto-char (point-min))
>           (while (re-search-forward ")[ \t\n]+)" nil t)
>             (setq found-gap-right (1+ found-gap-right))
>             (when (y-or-n-p (match-string 0)) ;"Delete gap? ")
>               (replace-match "))" nil nil nil 0)

That replace-match is too far away from re-search-forward. 
y-or-n-p may use search and replace too.
You should save the match-data with save-match-data:

           (while (re-search-forward ")[ \t\n]+)" nil t)
             (setq found-gap-right (1+ found-gap-right))
             (when (save-match-data (y-or-n-p (format "Delete gap %S?" (match-string 0))))
               (replace-match "))")))

And you don't need to pass the default values of optional parameters...


>               (setq inserted-gap-right (1+ inserted-gap-right)))))
>           (setq message (format "%sfound-gap-right: %s, inserted-gap-right: %s\n"
>                                 message
>                                 found-gap-right  inserted-gap-right))
>           (message message)
>         ))))

It is faster to call several times message than to concatenate strings
(even with format).  In emacs, buffer operations are more optimized
than string operations.

For this reason, and also because your function doesn't take into
account the rest of lisp syntax such as comments and strings, you
should rewrite it using higher level buffer walking functions such as
forward-sexp, looking-at, down-list, etc.

However there are a lot of edge cases (eg. we wouldn't allow spaces
between ' and the following sexp, or the dispatched macro character
and the following sexp #2A  (() ())  vs.   #2A(() ()); notably,
forward-sexp fails in the former case, it skips over #2A instead of
skipping over #2A  (() ()), but it works correctly over '  x.  On the
other hand, backward space fails in the ' x case, (but not for 'x)).

The problem here is that the emacs lisp reader is quite different from
the Common Lisp reader, and to deal correctly with Common Lisp, you
would have to deal with reader macros too.


-- 
__Pascal Bourguignon__                     http://www.informatimago.com/
Litter box not here.
You must have moved it again.
I'll poop in the sink.