unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Antonin Houska <ah@cybertec.at>
To: npostavs@users.sourceforge.net
Cc: 26049@debbugs.gnu.org
Subject: bug#26049: 25.2; Extra lines not added to comment
Date: Wed, 29 Mar 2017 08:54:53 +0200	[thread overview]
Message-ID: <20705.1490770493@localhost> (raw)
In-Reply-To: <87h92c3jl8.fsf@users.sourceforge.net>

npostavs@users.sourceforge.net wrote:

> Antonin Houska <ah@cybertec.at> 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.

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





  reply	other threads:[~2017-03-29  6:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-10 13:41 bug#26049: 25.2; Extra lines not added to comment Antonin Houska
2017-03-28  3:29 ` npostavs
2017-03-28 12:52   ` Antonin Houska
2017-03-29  2:25     ` npostavs
2017-03-29  6:54       ` Antonin Houska [this message]
2017-09-23  7:41         ` Antonin Houska
2017-09-23 14:37           ` Noam Postavsky
2017-11-06 10:14             ` Antonin Houska
2017-11-07  0:07               ` Noam Postavsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20705.1490770493@localhost \
    --to=ah@cybertec.at \
    --cc=26049@debbugs.gnu.org \
    --cc=npostavs@users.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).