Re: bug#3269: 23.0.93; C-mode text highlighting

Re: bug#3269: 23.0.93; C-mode text highlighting

[Alan Mackenzie]

Hello, Thomas and Yidong!

On Thu, May 14, 2009 at 09:39:24PM +0000, Alan Mackenzie wrote:
> On Wed, May 13, 2009 at 08:39:47AM +0200, Thomas Christensen wrote:

> > In c-mode type:
> > #define FOO "\
> > foo\n\
> > bar\n\
> > "

> > Then place the cursor after foo\n\ and press RETURN for a new line.

> > The highlighting is now broken, and I can only restore it by reverting
> > the buffer.

> That's a bit vague, so let me fill it out for you.  ;-)

> When you press RETURN as described, the new line you've just made lacks a
> backslash, hence terminates the macro.  (Yes, I know you knew that.)

>     #define FOO "\
>     foo\n\

>     bar\n\
>     "

> The "foo" line has lost its fontification, and this is the bug.  I know
> what's causing it, and it _might_ be easily fixable.  What's more, I
> don't this bug was in Emacs 22, so Chong Yidong would allow it to be
> fixed.  :-)

OK, here is a patch for half of the problem - it now fontifies a broken
string in a #define properly - just that you need to type M-o M-o after
the change.  Would you check that this works properly please, Thomas!

May I presume I can commit this to trunk before this week's pretest
release, Yidong?

The second half of the problem is to fix it so that you don't have to
type MoMo afterwards.  Any change you'll let me do this before this
week's pretest, Yidong?


2009-05-18  Alan Mackenzie  <bug-cc-mode@gnu.org>

	* progmodes/cc-fonts.el (c-font-lock-invalid-cpp-string-matcher):
	New function.
	(c-basic-matchers-before): New clause to fontify invalid strings
	in a CPP construct.

	* progmodes/cc-engine.el: Update some commenting.

	* progmodes/cc-defs.el (c-search-forward-char-property): new
	macro.


*** /home/acm/emacs/emacs.cvs/lisp/progmodes/cc-defs.el	2009-02-12 12:44:40.000000000 +0000
--- cc-defs.el	2009-05-18 10:55:56.716563912 +0000
***************
*** 1029,1034 ****
--- 1029,1053 ----
      ;; Emacs.
      `(remove-text-properties ,from ,to '(,property nil))))
  
+ (defmacro c-search-forward-char-property (property value &optional limit)
+   "Search forward for a text-property PROPERTY having value VALUE.
+ LIMIT bounds the search.  The comparison is done with `equal'.
+ 
+ Leave point just after the character, and set the match data on
+ this character, and return point.  If VALUE isn't found, Return
+ nil; point is then left undefined."
+   `(let ((place (point)))
+      (while
+ 	 (and
+ 	  (< place ,(or limit '(point-max)))
+ 	  (not (equal (get-text-property place ,property) ,value)))
+        (setq place (next-single-property-change
+ 		    place ,property nil ,(or limit '(point-max)))))
+      (when (< place ,(or limit '(point-max)))
+        (goto-char place)
+        (search-forward-regexp ".")	; to set the match-data.
+        (point))))
+ 
  (defun c-clear-char-property-with-value-function (from to property value)
    "Remove all text-properties PROPERTY from the region (FROM, TO)
  which have the value VALUE, as tested by `equal'.  These
*** /home/acm/emacs/emacs.cvs/lisp/progmodes/cc-engine.el	2009-02-21 20:19:30.000000000 +0000
--- cc-engine.el	2009-05-18 10:55:56.779554336 +0000
***************
*** 81,88 ****
  ;;
  ;; 'syntax-table
  ;;   Used to modify the syntax of some characters.  It is used to
! ;;   mark the "<" and ">" of angle bracket parens with paren syntax, and
! ;;   to "hide" obtrusive characters in preprocessor lines.
  ;;
  ;;   This property is used on single characters and is therefore
  ;;   always treated as front and rear nonsticky (or start and end open
--- 81,91 ----
  ;;
  ;; 'syntax-table
  ;;   Used to modify the syntax of some characters.  It is used to
! ;;   mark the "<" and ">" of angle bracket parens with paren syntax;
! ;;   also to "hide" obtrusive characters in preprocessor lines, by
! ;;   marking them with punctuation syntax, '(1).  If this value is ever
! ;;   used for any other purpose, modify
! ;;   `c-font-lock-invalid-cpp-string-matcher' accordingly.
  ;;
  ;;   This property is used on single characters and is therefore
  ;;   always treated as front and rear nonsticky (or start and end open
*** /home/acm/emacs/emacs.cvs/lisp/progmodes/cc-fonts.el	2009-01-05 03:23:18.000000000 +0000
--- cc-fonts.el	2009-05-18 10:55:56.887537920 +0000
***************
*** 285,291 ****
      ;; bit of the overhead compared to a real matcher.  The main reason
      ;; is however to pass the real search limit to the anchored
      ;; matcher(s), since most (if not all) font-lock implementations
!     ;; arbitrarily limits anchored matchers to the same line, and also
      ;; to insulate against various other irritating differences between
      ;; the different (X)Emacs font-lock packages.
      ;;
--- 285,291 ----
      ;; bit of the overhead compared to a real matcher.  The main reason
      ;; is however to pass the real search limit to the anchored
      ;; matcher(s), since most (if not all) font-lock implementations
!     ;; arbitrarily limit anchored matchers to the same line, and also
      ;; to insulate against various other irritating differences between
      ;; the different (X)Emacs font-lock packages.
      ;;
***************
*** 306,312 ****
      ;; covered by the font-lock context.)
  
      ;; Note: Replace `byte-compile' with `eval' to debug the generated
!     ;; lambda easier.
      (byte-compile
       `(lambda (limit)
  	(let (;; The font-lock package in Emacs is known to clobber
--- 306,312 ----
      ;; covered by the font-lock context.)
  
      ;; Note: Replace `byte-compile' with `eval' to debug the generated
!     ;; lambda more easily.
      (byte-compile
       `(lambda (limit)
  	(let (;; The font-lock package in Emacs is known to clobber
***************
*** 559,564 ****
--- 559,583 ----
  	     t)
  	   (c-put-font-lock-face start (1+ start) 'font-lock-warning-face)))))
  
+ (defun c-font-lock-invalid-cpp-string-matcher (lim)
+   ;; Fontify unterminated strings within preprocessor constructs.
+   ;;
+   ;; Unmatched string quotes will have been marked with a punctuation
+   ;; syntax-table text property (value '(1)) by `c-neutralize-syntax-in-CPP'.
+   ;; 
+   ;; This function will be called from font-lock for a region bounded by POINT
+   ;; and LIM, as though it were to identify a keyword for
+   ;; font-lock-keyword-face.  It always returns NIL to inhibit this and
+   ;; prevent a repeat invocation.  See elisp/lispref page "Search-based
+   ;; Fontification".
+   (while (c-search-forward-char-property 'syntax-table '(1) lim) ; punctuation
+     (when (memq (char-before) '(?\" ?\'))
+       (c-put-font-lock-face (1- (point)) (point) 'font-lock-warning-face)
+       (search-forward-regexp "\\(.*\\\\[\n\r]\\)*\\(.*$\\)")
+       (if (> (match-end 0) (match-beginning 0))
+ 	  (c-put-font-lock-face (match-beginning 0) (match-end 0)
+ 				'font-lock-string-face)))))
+ 
  (c-lang-defconst c-basic-matchers-before
    "Font lock matchers for basic keywords, labels, references and various
  other easily recognizable things that should be fontified before generic
***************
*** 580,585 ****
--- 599,611 ----
  	(concat ".\\(" c-string-limit-regexp "\\)")
  	'((c-font-lock-invalid-string)))
  
+       ;; Put a warning face on the opening quote of unclosed strings inside
+       ;; preprocessor construcs (#define).  The previous clause doesn't do
+       ;; this, since the hook function `c-neutralize-syntax-in-CPP' has
+       ;; splatted the syntax of the unmated string quotes.
+       ,@(when (c-lang-const c-opt-cpp-prefix)
+ 	  '((c-font-lock-invalid-cpp-string-matcher)))
+ 
        ;; Fontify keyword constants.
        ,@(when (c-lang-const c-constant-kwds)
  	  (let ((re (c-make-keywords-re nil (c-lang-const c-constant-kwds))))


-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#3269: 23.0.93; C-mode text highlighting

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

>> The "foo" line has lost its fontification, and this is the bug.  I know
>> what's causing it, and it _might_ be easily fixable.  What's more, I
>> don't this bug was in Emacs 22
>
> OK, here is a patch for half of the problem - it now fontifies a broken
> string in a #define properly - just that you need to type M-o M-o after
> the change.  Would you check that this works properly please, Thomas!
>
> The second half of the problem is to fix it so that you don't have to
> type MoMo afterwards.  Any change you'll let me do this before this
> week's pretest, Yidong?

Could you test these patches on some large C files, to make sure they
don't cause any additional performance problems?  But if everything
seems OK, please go ahead and check both patches in.

Re: bug#3269: 23.0.93; C-mode text highlighting

[Stefan Monnier]

> OK, here is a patch for half of the problem - it now fontifies a broken
> string in a #define properly

For some definition of "properly".

> The second half of the problem is to fix it so that you don't have to
> type M-o M-o afterwards.  Any change you'll let me do this before this
> week's pretest, Yidong?

I think this part is more important.  I don't care much (if at all)
about how invalid code is highlighted.


        Stefan

Re: bug#3269: 23.0.93; C-mode text highlighting

[Alan Mackenzie]

Hi, Yidong!

On Mon, May 18, 2009 at 12:41:21PM -0400, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> >> The "foo" line has lost its fontification, and this is the bug.  I
> >> know what's causing it, and it _might_ be easily fixable.  What's
> >> more, I don't this bug was in Emacs 22

> > OK, here is a patch for half of the problem - it now fontifies a
> > broken string in a #define properly - just that you need to type M-o
> > M-o after the change.  Would you check that this works properly
> > please, Thomas!

> > The second half of the problem is to fix it so that you don't have to
> > type MoMo afterwards.  Any change you'll let me do this before this
> > week's pretest, Yidong?

> Could you test these patches on some large C files, to make sure they
> don't cause any additional performance problems?  But if everything
> seems OK, please go ahead and check both patches in.

I've got both bits working now.  I'll do some more testing, and I expect
to be committing the whole caboodle withing 24 hours.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#3269: 23.0.93; C-mode text highlighting

[Alan Mackenzie]

Hi, Stefan!

On Mon, May 18, 2009 at 03:53:42PM -0400, Stefan Monnier wrote:
> > OK, here is a patch for half of the problem - it now fontifies a broken
> > string in a #define properly

> For some definition of "properly".

The opening string quote (?\" or ?\') gets f-l-warning-face.  The rest of
the unclosed string (up to the first EOL which isn't escaped) gets
f-l-string-face.

Actually, that's not _quite_ "proper".  A string with an even number of
backslashes at an EOL is broken at that point, but the font locking
doesn't show this (yet).  I don't suppose that will bother you all that
much.  ;-)

> > The second half of the problem is to fix it so that you don't have to
> > type M-o M-o afterwards.  Any change you'll let me do this before
> > this week's pretest, Yidong?

> I think this part is more important.  I don't care much (if at all)
> about how invalid code is highlighted.

I agree it's important.  I've got it working; what's more, the code
doesn't advise any of the font lock functions for (>= emacs-major-version
22).  I'll commit it tomorrow sometime.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#3269: 23.0.93; C-mode text highlighting

[Stefan Monnier]

>> > OK, here is a patch for half of the problem - it now fontifies a broken
>> > string in a #define properly
>> For some definition of "properly".
> The opening string quote (?\" or ?\') gets f-l-warning-face.  The rest of
> the unclosed string (up to the first EOL which isn't escaped) gets
> f-l-string-face.

> Actually, that's not _quite_ "proper".  A string with an even number of
> backslashes at an EOL is broken at that point, but the font locking
> doesn't show this (yet).  I don't suppose that will bother you all that
> much.  ;-)

I won't oppose the change, but just to be clear: I think that the
increased code complexity introduced by your patch is a worse problem
than the "improper" highlighting it tries to fix.

When code is syntactically incorrect, it's common/normal/expected for
the highlighting to be "incorrect".  This "incorrect" behavior is
actually a good way for the user to notice that his code has problems.
So, from this point of view, there's no need to highlight the opening
string quote with f-l-warning-face: just looking back in the buffer
until you find the first char that is not font-locked as expected will
find the culprit without any need for any extra elisp code, and moreover
this method will work in many more cases.

In other words, messed-up highlighting for incorrect code is just as
good if not better than explicitly recognizing the incorrect code and
highlighting it with f-l-warning-face.

When I introduced the use of f-l-warning-face in C strings, it was not
to avoid messed-up highlighting, but rather to avoid apparently correct
highlighting for code that was actually incorrect/unportable (and even
accepted by GCC at that time).

>> I think this part is more important.  I don't care much (if at all)
>> about how invalid code is highlighted.
> I agree it's important.  I've got it working; what's more, the code
> doesn't advise any of the font lock functions for (>= emacs-major-version
> 22).  I'll commit it tomorrow sometime.

Thanks,


        Stefan

Re: bug#3269: 23.0.93; C-mode text highlighting

[Alan Mackenzie]

Hi, Stefan,

On Mon, May 18, 2009 at 10:24:02PM -0400, Stefan Monnier wrote:
> > The opening string quote (?\" or ?\') gets f-l-warning-face.  The
> > rest of the unclosed string (up to the first EOL which isn't escaped)
> > gets f-l-string-face.

> > Actually, that's not _quite_ "proper".  A string with an even number of
> > backslashes at an EOL is broken at that point, but the font locking
> > doesn't show this (yet).  I don't suppose that will bother you all that
> > much.  ;-)

Whoops!  I was utterly wrong there.  When a string inside a #define has
an even number of backslashes at an EOL, this is perfectly legal; the
last \ escapes the EOL, concatenating the lines, and the second last \
escapes the first character on the next line.  Nice simple language, C.
;-)

> I won't oppose the change, but just to be clear: I think that the
> increased code complexity introduced by your patch is a worse problem
> than the "improper" highlighting it tries to fix.

Well, I don't agree with that, but I'm beginning to think that the
current fontification (ommitting f-l-string-face until the closing " is
there) wasn't perhaps quite so bad after all.

> When code is syntactically incorrect, it's common/normal/expected for
> the highlighting to be "incorrect".

Where "incorrect" here means "different from what it would be if the code
were correct".

> This "incorrect" behavior is actually a good way for the user to notice
> that his code has problems.

Agreed, totally.

> So, from this point of view, there's no need to highlight the opening
> string quote with f-l-warning-face: just looking back in the buffer
> until you find the first char that is not font-locked as expected will
> find the culprit without any need for any extra elisp code, and
> moreover this method will work in many more cases.

> In other words, messed-up highlighting for incorrect code is just as
> good if not better than explicitly recognizing the incorrect code and
> highlighting it with f-l-warning-face.

I was thinking of "compatibility" with unterminated strings in normal
code.  But they're not the same thing.  An open string in a #define is
perfectly valid code, if somewhat unusual outside of the Obfuscated C
competition.

You've persuaded me that the existing fontification is actually better.
So I won't be committing yesterday's patch.  Thanks!

I'll just finish the other patch and commit that.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#3269: 23.0.93; C-mode text highlighting

[Stefan Monnier]

>> In other words, messed-up highlighting for incorrect code is just as
>> good if not better than explicitly recognizing the incorrect code and
>> highlighting it with f-l-warning-face.

> I was thinking of "compatibility" with unterminated strings in normal
> code.  But they're not the same thing.  An open string in a #define is
> perfectly valid code, if somewhat unusual outside of the Obfuscated C
> competition.

> You've persuaded me that the existing fontification is actually better.
> So I won't be committing yesterday's patch.  Thanks!

I'm glad we ended up agreeing, tho for completely different reasons.

> I'll just finish the other patch and commit that.

Thanks,


        Stefan

Re: bug#3269: 23.0.93; C-mode text highlighting

[Alan Mackenzie]

Hi, All!

On Tue, May 19, 2009 at 10:36:05AM -0400, Stefan Monnier wrote:
> >> In other words, messed-up highlighting for incorrect code is just as
> >> good if not better than explicitly recognizing the incorrect code and
> >> highlighting it with f-l-warning-face.

> > I was thinking of "compatibility" with unterminated strings in normal
> > code.  But they're not the same thing.  An open string in a #define is
> > perfectly valid code, if somewhat unusual outside of the Obfuscated C
> > competition.

> > You've persuaded me that the existing fontification is actually better.
> > So I won't be committing yesterday's patch.  Thanks!

> I'm glad we ended up agreeing, tho for completely different reasons.

> > I'll just finish the other patch and commit that.

DONE.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#3269: 23.0.93; C-mode text highlighting

[Jim Meyering]

Hi,

Following up to this message,

    http://thread.gmane.org/gmane.emacs.devel/110963

I've included the trivially-rebased patch below, with a fixed log entry.
Also, have you considered offering XZ-compressed tarballs?
The decrease in size is striking:

    34M     emacs-23.0.93.tar.bz2
    26M     emacs-23.0.93.tar.xz

In addition, unpacking with xz takes about 1/3 the time, here:

    $ env time tar xf ../.new/emacs-23.0.93.tar.xz
    2.55user 0.83system 0:03.40elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
    0inputs+309000outputs (0major+17044minor)pagefaults 0swaps
    $ rm -rf emacs-23.0.93
    $ env time tar xf ../emacs-23.0.93.tar.bz2
    5.36user 0.83system 0:09.64elapsed 64%CPU (0avgtext+0avgdata 0maxresident)k
    67912inputs+309000outputs (0major+1448minor)pagefaults 0swaps

I admit that when I first compressed with xz, the resulting size was
27MiB, but when I reordered files in the tarball (placing like suffixes
together), xz was able to compress to one full MiB smaller.

From faa98d33239195f42c732f8886f1db378846553c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Mon, 18 May 2009 11:33:03 +0200
Subject: [PATCH] automatically handle .xz suffix (XZ-compressed files), too

* jka-cmpr-hook.el (jka-compr-compression-info-list): Add xz.
XZ is the successor to LZMA: <http://tukaani.org/xz/>
---
 lisp/jka-cmpr-hook.el |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lisp/jka-cmpr-hook.el b/lisp/jka-cmpr-hook.el
index 96e9513..fa2fd40 100644
--- a/lisp/jka-cmpr-hook.el
+++ b/lisp/jka-cmpr-hook.el
@@ -219,6 +219,10 @@ options through Custom does this automatically."
      "compressing"        "gzip"         ("-c" "-q")
      "uncompressing"      "gzip"         ("-c" "-q" "-d")
      t t "\037\213"]
+    ["\\.xz\\(~\\|\\.~[0-9]+~\\)?\\'"
+     "XZ compressing"     "xz"           ("-c" "-q")
+     "XZ uncompressing"   "xz"           ("-c" "-q" "-d")
+     t t "\3757zXZ\0"]
     ;; dzip is gzip with random access.  Its compression program can't
     ;; read/write stdin/out, so .dz files can only be viewed without
     ;; saving, having their contents decompressed with gzip.
--
1.6.3.1.149.gbc70c