bug#24767: jgraph comments not recognized any more

bug#24767: jgraph comments not recognized any more

[Stefan Monnier]

Package: Emacs


Try the following:

    % emacs -Q -l .../elpa/packages/jgraph-mode/jgraph-mode.el ~/foo.jgr
    (* hello *)

The string "(* hello *)" should be highlighted as a comment, and is
indeed correctly highlighted this way in Emacs-25 (and Emacs-24), but
not in "master".

The particularity of jgraph-mode's handling of (*...*) is that ?(, ?),
and ?* are given "symbol" syntax, so I guess that src/syntax.c was
modified in a way which makes it skip the whole (* as being a symbol.


        Stefan

bug#24767: jgraph comments not recognized any more

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 22 Oct 2016 17:24:01 -0400
> 
>     % emacs -Q -l .../elpa/packages/jgraph-mode/jgraph-mode.el ~/foo.jgr
>     (* hello *)
> 
> The string "(* hello *)" should be highlighted as a comment, and is
> indeed correctly highlighted this way in Emacs-25 (and Emacs-24), but
> not in "master".
> 
> The particularity of jgraph-mode's handling of (*...*) is that ?(, ?),
> and ?* are given "symbol" syntax, so I guess that src/syntax.c was
> modified in a way which makes it skip the whole (* as being a symbol.

I suspect 9dcf599.  Alan, could you please take a look?

Stefan, I think it would be good to have a test for this in the test
suite, even if for now it will fail.

Thanks.

bug#24767: jgraph comments not recognized any more

[Alan Mackenzie]

Hello, Eli.

On Sun, Oct 23, 2016 at 09:17:19AM +0300, Eli Zaretskii wrote:
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Date: Sat, 22 Oct 2016 17:24:01 -0400
> > 
> >     % emacs -Q -l .../elpa/packages/jgraph-mode/jgraph-mode.el ~/foo.jgr
> >     (* hello *)
> > 
> > The string "(* hello *)" should be highlighted as a comment, and is
> > indeed correctly highlighted this way in Emacs-25 (and Emacs-24), but
> > not in "master".
> > 
> > The particularity of jgraph-mode's handling of (*...*) is that ?(, ?),
> > and ?* are given "symbol" syntax, so I guess that src/syntax.c was
> > modified in a way which makes it skip the whole (* as being a symbol.

> I suspect 9dcf599.  Alan, could you please take a look?

Yes, 9dcf599 is indeed where the bug was introduced.  I'll be looking at
it.

> Stefan, I think it would be good to have a test for this in the test
> suite, even if for now it will fail.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#24767: jgraph comments not recognized any more

[Alan Mackenzie]

Hello, Stefan.

On Sat, Oct 22, 2016 at 05:24:01PM -0400, Stefan Monnier wrote:
> Package: Emacs


> Try the following:

>     % emacs -Q -l .../elpa/packages/jgraph-mode/jgraph-mode.el ~/foo.jgr
>     (* hello *)

> The string "(* hello *)" should be highlighted as a comment, and is
> indeed correctly highlighted this way in Emacs-25 (and Emacs-24), but
> not in "master".

> The particularity of jgraph-mode's handling of (*...*) is that ?(, ?),
> and ?* are given "symbol" syntax, so I guess that src/syntax.c was
> modified in a way which makes it skip the whole (* as being a symbol.

Suppose the jgraph buffer contains:

    he(*llo*)

.  Is this supposed to analyse as the symbol "he" followed by a comment,
or should it be the symbol "he(*llo*)"?  Currently, even Emacs-25
doesn't recognise the "(*llo*)" as a comment.  It seems to me more
likely that the comment should be recognised, but I don't know jgraph.

To blame for this is the section of code in scan_sexps_forward (in
Emacs-25) just after the label symstarted: where there is a subsidiary
loop on "(from < end)" which doesn't do any checking on the comment
flags.

Maybe the solution (in master) would be to add the checking of the
comment flags into this subsidiary loop.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#24767: jgraph comments not recognized any more

[Stefan Monnier]

> Suppose the jgraph buffer contains:
>
>     he(*llo*)
>
> .  Is this supposed to analyse as the symbol "he" followed by a comment,
> or should it be the symbol "he(*llo*)"?

Good question.  To the extent that src/syntax.c shouldn't be specific to
jgraph-mode, the answer shouldn't depend on the choice made by
Jgraph's author.

> Currently, even Emacs-25 doesn't recognise the "(*llo*)" as a comment.

I think it's good enough to preserve backward compatibility, then.

> It seems to me more likely that the comment should be recognised, but
> I don't know jgraph.

To choose which of the two behavior is desired, the major mode author
can use syntax-propertize to catch this rare corner case anyway.

> Maybe the solution (in master) would be to add the checking of the
> comment flags into this subsidiary loop.

Let's not worry about it.


        Stefan

bug#24767: jgraph comments not recognized any more

[Alan Mackenzie]

Hello, Stefan.

On Tue, Oct 25, 2016 at 09:52:28AM -0400, Stefan Monnier wrote:
> > Suppose the jgraph buffer contains:
> >
> >     he(*llo*)
> >
> > .  Is this supposed to analyse as the symbol "he" followed by a comment,
> > or should it be the symbol "he(*llo*)"?

> Good question.  To the extent that src/syntax.c shouldn't be specific to
> jgraph-mode, the answer shouldn't depend on the choice made by
> Jgraph's author.

> > Currently, even Emacs-25 doesn't recognise the "(*llo*)" as a comment.

> I think it's good enough to preserve backward compatibility, then.

> > It seems to me more likely that the comment should be recognised, but
> > I don't know jgraph.

> To choose which of the two behavior is desired, the major mode author
> can use syntax-propertize to catch this rare corner case anyway.

> > Maybe the solution (in master) would be to add the checking of the
> > comment flags into this subsidiary loop.

> Let's not worry about it.

In the end, that's what I've done.  The fix was less tricky than I
thought it would be.  It recognises the comment in he(*llo*).

Would you try out the following patch, please, and let me know whether
there are still problems with it.  Thanks.



diff --git a/src/syntax.c b/src/syntax.c
index 667de40..d463f7e 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -3124,6 +3124,7 @@ scan_sexps_forward (struct lisp_parse_state *state,
   ptrdiff_t prev_from;		/* Keep one character before FROM.  */
   ptrdiff_t prev_from_byte;
   int prev_from_syntax, prev_prev_from_syntax;
+  int syntax;
   bool boundary_stop = commentstop == -1;
   bool nofence;
   bool found;
@@ -3191,8 +3192,6 @@ do { prev_from = from;				\
 
   while (from < end)
     {
-      int syntax;
-
       if (SYNTAX_FLAGS_COMSTART_FIRST (prev_from_syntax)
 	  && (c1 = FETCH_CHAR (from_byte),
 	      syntax = SYNTAX_WITH_FLAGS (c1),
@@ -3258,7 +3257,24 @@ do { prev_from = from;				\
 	  while (from < end)
 	    {
 	      int symchar = FETCH_CHAR_AS_MULTIBYTE (from_byte);
-	      switch (SYNTAX (symchar))
+
+              if (SYNTAX_FLAGS_COMSTART_FIRST (prev_from_syntax)
+                  && (syntax = SYNTAX_WITH_FLAGS (symchar),
+                      SYNTAX_FLAGS_COMSTART_SECOND (syntax)))
+                {
+                  state->comstyle
+                    = SYNTAX_FLAGS_COMMENT_STYLE (syntax, prev_from_syntax);
+                  comnested = (SYNTAX_FLAGS_COMMENT_NESTED (prev_from_syntax)
+                               | SYNTAX_FLAGS_COMMENT_NESTED (syntax));
+                  state->incomment = comnested ? 1 : -1;
+                  state->comstr_start = prev_from;
+                  INC_FROM;
+                  prev_from_syntax = Smax;
+                  code = Scomment;
+                  goto atcomment;
+                }
+
+              switch (SYNTAX (symchar))
 		{
 		case Scharquote:
 		case Sescape:
@@ -3280,6 +3296,7 @@ do { prev_from = from;				\
 
 	case Scomment_fence: /* Can't happen because it's handled above.  */
 	case Scomment:
+        atcomment:
           if (commentstop || boundary_stop) goto done;
 	startincomment:
 	  /* The (from == BEGV) test was to enter the loop in the middle so


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#24767: jgraph comments not recognized any more

[Stefan Monnier]

> Would you try out the following patch, please, and let me know whether
> there are still problems with it.  Thanks.

I confirm it fixes my problems, thank you,


        Stefan

bug#24767: jgraph comments not recognized any more

[Alan Mackenzie]

Hello, Stefan.

On Sat, Oct 29, 2016 at 11:12:51AM -0400, Stefan Monnier wrote:
> > Would you try out the following patch, please, and let me know whether
> > there are still problems with it.  Thanks.

> I confirm it fixes my problems, thank you,

I've committed it, and am closing the bug.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).