Re: check-lisp-parenthesis

[Cecil Westerhof <Cecil@decebal.nl>]

pjb@informatimago.com (Pascal J. Bourguignon) writes:

>> I started with a function to check lisp parenthesis.
>
> What's wrong with check-parens?

Wrong name. It does not check for matching, but if there are not the
wrong characters before/after a parenthesis.

But maybe not a bad idea to do a syntax check also. But can not be just
like that, because calling it stops the function when there is an
imbalance.


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

First I only wrote a function to check my files, after you told me about
the allowed characters before/after a parenthesis. But when written I
extended it to insert spaces where they are missing. But when the
functionality changes, the name should change also of course. ;-)


>> 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 " "))

There is. Using your code on the program itself, would break the code
(inserting spaces in the regular expressions).


>>           (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 "))")))

Works like a charm. Thanks.
I also implemented the deleting of the left gaps.


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

I was playing with them, because I thought that the problem could be
there. Should have deleted them.


>>               (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.

I find clean code more important then optimum speed (especially when
using interactive). Also, I want all
the messages at once.


> 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.

That is why interactive is a good option. Then can the user decide. This
function is at least a lot nicer then having to do all the checks by
hand. ;-)

An updated version is in own-functions-general.el on:
    http://www.decebal.nl//EmacsLisp/

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof