From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: scratch/fontify-open-string. [Was: CC Mode and electric-pair "problem".] Date: Sun, 15 Jul 2018 18:45:38 +0000 Message-ID: <20180715184538.GC4897@ACM> References: <20180627182717.GA4625@ACM> <20180630190327.GC6816@ACM> <83tvpkkr93.fsf@gnu.org> <20180630201447.GE6816@ACM> <83o9frkmk7.fsf@gnu.org> <20180701163825.GC4697@ACM> <86bmbiceq7.fsf@stephe-leake.org> <86lgacswk8.fsf@stephe-leake.org> <83bmb8a5x0.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1531680949 25939 195.159.176.226 (15 Jul 2018 18:55:49 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 15 Jul 2018 18:55:49 +0000 (UTC) User-Agent: Mutt/1.9.4 (2018-02-28) Cc: Stephen Leake , emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Jul 15 20:55:45 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1femBM-0006Za-CS for ged-emacs-devel@m.gmane.org; Sun, 15 Jul 2018 20:55:44 +0200 Original-Received: from localhost ([::1]:47180 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1femDT-0003I7-D3 for ged-emacs-devel@m.gmane.org; Sun, 15 Jul 2018 14:57:55 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:48059) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1femDJ-0003I0-8k for emacs-devel@gnu.org; Sun, 15 Jul 2018 14:57:50 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1femDF-0005LI-0x for emacs-devel@gnu.org; Sun, 15 Jul 2018 14:57:45 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:55000 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1femDC-0005Hp-Mm for emacs-devel@gnu.org; Sun, 15 Jul 2018 14:57:39 -0400 Original-Received: (qmail 48134 invoked by uid 3782); 15 Jul 2018 18:57:34 -0000 Original-Received: from acm.muc.de (p5B14752F.dip0.t-ipconnect.de [91.20.117.47]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 15 Jul 2018 20:57:34 +0200 Original-Received: (qmail 6687 invoked by uid 1000); 15 Jul 2018 18:45:38 -0000 Content-Disposition: inline In-Reply-To: <83bmb8a5x0.fsf@gnu.org> X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x [fuzzy] X-Received-From: 193.149.48.1 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:227438 Archived-At: Hello, Eli, thanks for the review. The code is still preliminary, and is missing quite a lot of comments, still. I have had doubts about the mechanism (e.g. C-M-b will take a lot of work to make it functional), see my reply to Stephen. On Sun, Jul 15, 2018 at 18:13:15 +0300, Eli Zaretskii wrote: > > From: Stephen Leake > > Date: Sun, 15 Jul 2018 04:00:23 -0500 > > Anything I can do to help merge this to main? > A few things: > . NEWS > . Updates for the relevant parts in the manual(s) > . Minor nits below: > > +(defcustom font-lock-warn-open-string t > > + "Fontify the opening quote of an unterminated string with warning face? > > +This is done when this variable is non-nil. > We use a slightly different style for such options (slightly rephrased > to fit on one line): Well done for the compression! > "Non-nil means show opening quotes of unterminated strings with warning face." > > +This works only when the syntax-table entry for newline contains the flag `s' > > +\(see page \"xxx\" in the Elisp manual)." > Please replace "xxx" with an actual value. Also, we don't refer to > our manuals as "pages", that is a relic from the "man pages" era. Yes, thanks. Just "see \"\"", without the "page"? > > +#define DEC_AT \ > Please #undef DEC_AT when you are done using it (at function's end). OK. > > + /* Find the alleged string opener. */ > Please leave 2 spaces between the end of the comment and "*/" (here > and elsewhere in the patch) OK. As a matter of interest, what is the reason for this? I've seen it all over the Emacs C code. Is it something to do with filling? > > + while ((at > stop) > > + && (code != Sstring) > > + && (!SYNTAX_FLAGS_CLOSE_STRING (syntax))) > > + { > > + DEC_AT; > > + } > A single line doesn't need braces. I'm intending to put more code in there. > > + /* Search back for a terminating string delimiter: */ > > + while ((at > stop) > > + && (code != Sstring) > > + && (code != Sstring_fence) > > + && (!SYNTAX_FLAGS_CLOSE_STRING (syntax))) > > + { > > + DEC_AT; > > + /* Check for comment and "other" strings. */ > > + } > Is the last comment at its correct place? It doesn't seem to refer to > any code. It's a FIXME: "Put in code here to check for comment and "other" strings.". > > + lose: > > + UPDATE_SYNTAX_TABLE_FORWARD (*from); > > + return false; > > + > > + lossage: > > + /* We've encountered possible comments or strings with mixed > > + delimiters. Bail out and scan forward from a safe position. */ > "lose" and "lossage" are too similar. Can we have a better name for > the latter? OK. I took the names from, I think, back_comment. > > + { > > + struct lisp_parse_state state; > > + bool adjusted = true; > Why did you need the braces here? C99 allows to mix declarations and > statements, so we no longer need such braces. OK. > > + find_start_value > > + = CONSP (state.levelstarts) ? XINT (XCAR (state.levelstarts)) > > + : state.thislevelstart >= 0 ? state.thislevelstart > > + : find_start_value; > Please use parentheses here for better readability (to clearly show > which parts belong to which condition). Yes, it didn't indent well by itself. Maybe I should raise this with the CC Mode maintainer. But yes, I'll put parens in. > > -Comments are ignored if `parse-sexp-ignore-comments' is non-nil. > > +Comments are skipped over if `parse-sexp-ignore-comments' is non-nil. > We nowadays prefer to quote 'like this' in comments and plain text. OK. > > -Comments are ignored if `parse-sexp-ignore-comments' is non-nil. > > +Comments are skipped over if `parse-sexp-ignore-comments' is non-nil. > Likewise. > Thanks again for working on this. I'll make the stylistic corrections, then get working on it again in earnest. -- Alan Mackenzie (Nuremberg, Germany).