Re: first steps in elisp

[Joost Kremers]

On Thu, Nov 24 2016, Mark Piffer wrote:
> (defun ignore-multiline-comment (nlines)

It's a good habit to get into to always provide a prefix for any 
Elisp functions and variables that you write, even if it's just 
for your own personal code. For personal code, most people use 
their initials, or simply `my-`.

>   "assumes point is inside a C multiline comment /*. Advances
> until end of comment */ or nlines becomes 0"

There are some doc string conventions (which can be found in the 
Elisp manual: C-h i m Elisp RET m Documentation Tips RET), the 
most important of which is that the first line of a doc string 
should be a sentence on its own. For personal code, it doesn't 
matter as much, of course, but again, it's a good habit to get 
into. Also, the doc string should say what a function does, i.e., 
what it expects as input (or context) and what it returns or what 
side effects it has. So, e.g.,:

     "Move point past the end of a C multiline comment.
This function assumes that point is inside the comment.  This is a 
recursive function, its stop condition is met when NLINES is 0."

>   (if (zerop nlines)
>       nil
>     (if (looking-at ".*?\\*/")
>                 (progn
>                   (goto-char (match-end 0))
>                   (ignore-line-comments nlines))
>       (beginning-of-line 2)
>       (ignore-multiline-comment (1- nlines)))))

Personally, I normally wouldn't explicitly write nil as a return 
value but instead rely on the function structure to handle that. 
So I'd do something like:


```
(defun my-ignore-multiline-comment (nlines)
 "assumes point is inside a C multiline comment /*. Advances
until end of comment */ or nlines becomes 0"
 (when (not (zerop nlines))
   (if (looking-at ".*?\\*/")
       (progn
         (goto-char (match-end 0))
         (my-ignore-line-comments nlines))
     (beginning-of-line 2)
     (my-ignore-multiline-comment (1- nlines)))))
```

But in this case the nil return value might make sense to indicate 
that the stop condition is met. Personally, I would probably write 
that in the doc string, but YMMV.

> (defun ignore-line-comments (nlines)
>   "return the text starting at point as a list, going nlines 
>   lines down, stripped of
> all C comments (except pathological cases w/ string literals)"
>   (if (zerop nlines)
>       nil
>     (setq ml-e (if (looking-at "\\(.*?\\)/\\*") ;; test on /* 
>     comment
>                                  (match-end 1)
>                                nil))
>     (setq sl-e (if (looking-at "\\(.*?\\)//") ;; test on // 
>     comment
>                                   (match-end 1)
>                                nil))

Here, you should definitely use local variables for ml-e and sl-e. 
Note that if you use setq on a symbol that hasn't been let-bound, 
you automatically create a global variable that stays around until 
Emacs is shut down. So unless you need to store some global state, 
always use let.

Also, I would use longer names for variables, so it's easier to 
see what they're used for. But again, YMMV.

>     (if (or sl-e ml-e) ;; any comment on line?
>                 (if (and ml-e (or (not sl-e) (< ml-e sl-e)))  ;; 
>                 is /* the only or first comment?
>                     (progn
>                       (setq r (buffer-substring-no-properties 
>                       (point) ml-e))
>                       (goto-char ml-e)
>                       (cons r (ignore-multiline-comment 
>                       nlines)))
>                  (setq r (buffer-substring-no-properties (point) 
>                  sl-e))
>                   (beginning-of-line 2)
>                   (cons r (ignore-line-comments (1- nlines))))
>       (looking-at ".*$")
>       (setq r (buffer-substring-no-properties (car (match-data)) 
>       (cadr (match-data))) )
>       (beginning-of-line 2)
>       (cons r (ignore-line-comments (1- nlines))))))

So with let, this would become:

```
(defun my-ignore-line-comments (nlines)
 "return the text starting at point as a list, going nlines lines 
 down, stripped of 
all C comments (except pathological cases w/ string literals)"
 (when (not (zerop nlines)) 
   (let ((ml-e (if (looking-at "\\(.*?\\)/\\*") ;; test on /* 
   comment
                   (match-end 1)
                 nil))
         (sl-e (if (looking-at "\\(.*?\\)//") ;; test on // 
         comment
                   (match-end 1)
                 nil))
         r)
     (if (or sl-e ml-e) ;; any comment on line?
         (if (and ml-e (or (not sl-e) (< ml-e sl-e)))  ;; is /* 
         the only or first comment?
             (progn
               (setq r (buffer-substring-no-properties (point) 
               ml-e))
               (goto-char ml-e)
               (cons r (my-ignore-multiline-comment nlines)))
           (setq r (buffer-substring-no-properties (point) sl-e))
           (beginning-of-line 2)
           (cons r (my-ignore-line-comments (1- nlines))))
       (looking-at ".*$")
       (setq r (buffer-substring-no-properties (car (match-data)) 
       (cadr (match-data))))
       (beginning-of-line 2)
       (cons r (my-ignore-line-comments (1- nlines)))))))
```

It's perfectly fine to use setq on a variable once it's been 
let-bound. If you cannot provide an initial value for a variable, 
just leave out the value as done here for `r` and setq it later.

BTW, I find it useful to run flycheck in my Elisp buffers, helps 
one to develop good habits. :-)

HTH 

--
Joost Kremers
Life has its moments