check-lisp-parenthesis

check-lisp-parenthesis

[Cecil Westerhof]

I started with a function to check lisp parenthesis.

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

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

Re: check-lisp-parenthesis

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

Re: check-lisp-parenthesis

[Cecil Westerhof]

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