unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: npostavs@users.sourceforge.net
Cc: 24870@debbugs.gnu.org, Matt Armstrong <marmstrong@google.com>
Subject: bug#24870: 26.0.50; parse-partial-sexp ignores comment-end
Date: Thu, 29 Dec 2016 11:14:28 +0000	[thread overview]
Message-ID: <20161229111428.GA12532@acm.fritz.box> (raw)
In-Reply-To: <8737hldce8.fsf@users.sourceforge.net>

Hello, Noam.

Sorry this has taken me more time than I anticipated; it's a busy time
of the year.  :-(

On Sun, Dec 18, 2016 at 12:39:11AM -0500, npostavs@users.sourceforge.net wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hello, Noam.

> > On Thu, Dec 15, 2016 at 11:33:36AM -0500, Noam Postavsky wrote:

[ .... ]

> >> > Do you want to make this fix, or should I do it?  If you want to
> >> > do it, I'm willing (indeed, eager) to review it for you.

> >> I'll have a patch ready in a day or two.

[ .... ]

> Okay, here it is.  I think I've made this function a bit less twisty,
> and hopefully haven't broken anything new (make check is still passing).

syntax.c looks good.  It looks very good.  I've just got one trivial
comment:

(i) The new function `check_comment_start' doesn't have a comment saying
what its return value means.  Possibly you could instead rename it so
that the name implies what it returns.  Maybe something like
`in_double_comment_opener'.

I'll admit I haven't actually tried out the code, mainly because you've
written a test file.  One comment about the test file:

(ii) In `parse-partial-sexp-continue-over-comment-marker', variable aftC
is the position in the middle of the comment closer "*/".  I don't think
you are testing in any way that element 10 (nil, or the syntax of the
position just before the end point when that position might be the first
character of a two-character construct, i.e. an escape or first char of a
double-char comment delimiter) is correct.  This element 10 was newly
introduced this year.  Would you please consider adding such a test to
this new test file.  Thanks!

Otherwise, excellent!

-- 
Alan Mackenzie (Nuremberg, Germany).





  reply	other threads:[~2016-12-29 11:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-03 19:38 bug#24870: 26.0.50; parse-partial-sexp ignores comment-end Andreas Röhler
2016-11-30  9:10 ` Matt Armstrong
2016-11-30 12:37   ` Andreas Röhler
2016-11-30 23:02     ` Matt Armstrong
2016-12-01  1:17       ` npostavs
2016-12-01  8:24         ` Andreas Röhler
2016-12-14  3:00           ` npostavs
2016-12-14  4:04             ` npostavs
2016-12-14  6:45               ` Andreas Röhler
2016-12-14 19:56               ` Alan Mackenzie
2016-12-15  8:18                 ` Andreas Röhler
2016-12-15 16:01                   ` Eli Zaretskii
2016-12-15 16:50                   ` Alan Mackenzie
2016-12-15 17:59                     ` Andreas Röhler
2016-12-14 21:58               ` Alan Mackenzie
2016-12-15 16:33                 ` Noam Postavsky
2016-12-15 16:44                   ` Alan Mackenzie
2016-12-18  5:39                     ` npostavs
2016-12-29 11:14                       ` Alan Mackenzie [this message]
2016-12-30  1:55                         ` npostavs
2017-01-13  2:07                           ` npostavs
2017-01-23 20:12                             ` Alan Mackenzie
2017-01-24  0:30                               ` npostavs
2016-12-01  8:33       ` Andreas Röhler

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=20161229111428.GA12532@acm.fritz.box \
    --to=acm@muc.de \
    --cc=24870@debbugs.gnu.org \
    --cc=marmstrong@google.com \
    --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).