Subject: bug#26049: 25.2; Extra lines not added to comment

From: Antonin Houska <[email-masked]>
Date: Sat, 23 Sep 2017 09:41:37 +0200

Antonin Houska <[email-masked]> wrote:

> [email-masked] wrote:
> 
> > Antonin Houska <[email-masked]> writes:
> > >
> > >> >   	   (progn (goto-char end) (end-of-line) (skip-syntax-backward " ")
> > >> >   		  (<= (point) end))
> > >> >   	   (or block (not (string= "" comment-end)))
> > >> > ! 	   (or block (progn (goto-char beg) (search-forward
> > >> > !                                              "\n"
> > >> > !                                              (min (1+ end) (point-max)) t)))))
> > >
> > >> Maybe (re-search-forward "$" end t) is better?  It's a bit unclear to me
> > >> what exactly all those tests are looking for.  That code could use some
> > >> comments...
> > >
> > > I've just verified your approach - it does work too.
> 
> > "$" also matches at the end of buffer even if it doesn't end in newline
> > (which is a very marginal corner case, I just happened to notice it
> > because I didn't hit RET in my test buffer).
> 
> IMO this is ok. If the 'extra-line value of `comment-style' tells that the
> comment should look like this
> 
> /*
>  * some comment
>  */
> 
> it'd be kind of inconsistend if just a missing RET at the end of buffer
> resulted in this
> 
> /* some comment */
> 
> which effectively means discrepancy from the customization setting.
> 
> (The initial version of my patch ignored the `extra-line' setting in this
> special case, but it was a thinko rather than intention.)
> 
> > 
> > > + 	    ;; Trim trailing whitespace from cs if there's some.
> > > +             (setq cs (string-trim cs))
> > 
> > This would trim leading whitespace too, do we want that?
> 
> I haven't noticed any related issue but yes, string-trim-right is more
> precise. If the (supposedly accidental) leading space should be removed from
> the value of `comment-start', it should probably happen elsewhere in the code
> because it's not specific to the 'extra-line style.

The next version of the patch (with string-trim replaced with
string-trim-right) is below. Is there anything else I should do?

-- 
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
new file mode 100644
index 1af8929..f2d9735
*** a/lisp/newcomment.el
--- b/lisp/newcomment.el
***************
*** 69,74 ****
--- 69,77 ----
  
  ;;; Code:
  
+ (eval-when-compile
+   (require 'subr-x))
+ 
  ;;;###autoload
  (defalias 'indent-for-comment 'comment-indent)
  ;;;###autoload
*************** the region rather than at left margin."
*** 1139,1144 ****
--- 1142,1150 ----
  
  	  ;; make the leading and trailing lines if requested
  	  (when lines
+ 	    ;; Trim trailing whitespace from cs if there's some.
+             (setq cs (string-trim-right cs))
+ 
  	    (let ((csce
  		   (comment-make-extra-lines
  		    cs ce ccs cce min-indent max-indent block)))
*************** changed with `comment-style'."
*** 1209,1215 ****
  	   (progn (goto-char end) (end-of-line) (skip-syntax-backward " ")
  		  (<= (point) end))
  	   (or block (not (string= "" comment-end)))
! 	   (or block (progn (goto-char beg) (search-forward "\n" end t)))))
  
      ;; don't add end-markers just because the user asked for `block'
      (unless (or lines (string= "" comment-end)) (setq block nil))
--- 1215,1221 ----
  	   (progn (goto-char end) (end-of-line) (skip-syntax-backward " ")
  		  (<= (point) end))
  	   (or block (not (string= "" comment-end)))
! 	   (or block (progn (goto-char beg) (re-search-forward "$" end t)))))
  
      ;; don't add end-markers just because the user asked for `block'
      (unless (or lines (string= "" comment-end)) (setq block nil))