RFR cc-langs.el: change syntax for @ in Java

RFR cc-langs.el: change syntax for @ in Java

[Filipp Gunbin]

Hi Alan, are you ok if I make this small change?  Prefix is better
syntax for @ in Java, because it just denotes that the following is
an annotation interface name (but is not part of it).

Thanks.
Filipp

diff --git a/lisp/progmodes/cc-langs.el b/lisp/progmodes/cc-langs.el
index fa4e73087e..e2c1c6099f 100644
--- a/lisp/progmodes/cc-langs.el
+++ b/lisp/progmodes/cc-langs.el
@@ -378,12 +378,14 @@ c-make-mode-syntax-table
        (let ((table (make-syntax-table)))
 	 (c-populate-syntax-table table)
 	 ;; Mode specific syntaxes.
-	 ,(cond ((or (c-major-mode-is 'objc-mode) (c-major-mode-is 'java-mode))
+	 ,(cond ((c-major-mode-is 'objc-mode)
 		 ;; Let '@' be part of symbols in ObjC to cope with
 		 ;; its compiler directives as single keyword tokens.
 		 ;; This is then necessary since it's assumed that
 		 ;; every keyword is a single symbol.
 		 '(modify-syntax-entry ?@ "_" table))
+		((c-major-mode-is 'java-mode)
+		 '(modify-syntax-entry ?@ "'" table))
 		((c-major-mode-is 'pike-mode)
 		 '(modify-syntax-entry ?@ "." table)))
 	 table)))

Re: RFR cc-langs.el: change syntax for @ in Java

[Alan Mackenzie]

Hello, Fillip.

On Wed, Apr 07, 2021 at 23:41:09 +0300, Filipp Gunbin wrote:
> Hi Alan, are you ok if I make this small change?  Prefix is better
> syntax for @ in Java, because it just denotes that the following is
> an annotation interface name (but is not part of it).

Do you have a specific test case where things go wrong at the moment, or
work better with the change in place?

I'll run the CC Mode test suite with this change to see how it goes.  It
might entail changes to functions like c-forward-decl-or-cast-1.

> Thanks.
> Filipp

> diff --git a/lisp/progmodes/cc-langs.el b/lisp/progmodes/cc-langs.el
> index fa4e73087e..e2c1c6099f 100644
> --- a/lisp/progmodes/cc-langs.el
> +++ b/lisp/progmodes/cc-langs.el
> @@ -378,12 +378,14 @@ c-make-mode-syntax-table
>         (let ((table (make-syntax-table)))
>  	 (c-populate-syntax-table table)
>  	 ;; Mode specific syntaxes.
> -	 ,(cond ((or (c-major-mode-is 'objc-mode) (c-major-mode-is 'java-mode))
> +	 ,(cond ((c-major-mode-is 'objc-mode)
>  		 ;; Let '@' be part of symbols in ObjC to cope with
>  		 ;; its compiler directives as single keyword tokens.
>  		 ;; This is then necessary since it's assumed that
>  		 ;; every keyword is a single symbol.
>  		 '(modify-syntax-entry ?@ "_" table))
> +		((c-major-mode-is 'java-mode)
> +		 '(modify-syntax-entry ?@ "'" table))
>  		((c-major-mode-is 'pike-mode)
>  		 '(modify-syntax-entry ?@ "." table)))
>  	 table)))

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: RFR cc-langs.el: change syntax for @ in Java

[Alan Mackenzie]

Hello again, Filipp

On Wed, Apr 07, 2021 at 23:41:09 +0300, Filipp Gunbin wrote:
> Hi Alan, are you ok if I make this small change?  Prefix is better
> syntax for @ in Java, because it just denotes that the following is
> an annotation interface name (but is not part of it).

I've just run the CC Mode test suite with this amendment, and it
produces the following successes on annotations-1.java:

annotations-1.java:4: Expected analysis ((inclass 1) (topmost-intro 21)), got ((inclass 21) (topmost-intro 21))
annotations-1.java:4: Expected indentation 0, got 32
annotations-1.java:5: Expected analysis ((class-close 1)), got ((class-close 21))

..  So, there are things to sort out before we can apply the change.
Again, what's the reason for this change?  There must be a test case
where the current CC Mode handles some Java source code badly.

> Thanks.
> Filipp

> diff --git a/lisp/progmodes/cc-langs.el b/lisp/progmodes/cc-langs.el
> index fa4e73087e..e2c1c6099f 100644
> --- a/lisp/progmodes/cc-langs.el
> +++ b/lisp/progmodes/cc-langs.el
> @@ -378,12 +378,14 @@ c-make-mode-syntax-table
>         (let ((table (make-syntax-table)))
>  	 (c-populate-syntax-table table)
>  	 ;; Mode specific syntaxes.
> -	 ,(cond ((or (c-major-mode-is 'objc-mode) (c-major-mode-is 'java-mode))
> +	 ,(cond ((c-major-mode-is 'objc-mode)
>  		 ;; Let '@' be part of symbols in ObjC to cope with
>  		 ;; its compiler directives as single keyword tokens.
>  		 ;; This is then necessary since it's assumed that
>  		 ;; every keyword is a single symbol.
>  		 '(modify-syntax-entry ?@ "_" table))
> +		((c-major-mode-is 'java-mode)
> +		 '(modify-syntax-entry ?@ "'" table))
>  		((c-major-mode-is 'pike-mode)
>  		 '(modify-syntax-entry ?@ "." table)))
>  	 table)))

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: RFR cc-langs.el: change syntax for @ in Java

[Filipp Gunbin]

On 08/04/2021 10:15 +0000, Alan Mackenzie wrote:

> I've just run the CC Mode test suite with this amendment, and it
> produces the following successes on annotations-1.java:
>
> annotations-1.java:4: Expected analysis ((inclass 1) (topmost-intro 21)), got ((inclass 21) (topmost-intro 21))
> annotations-1.java:4: Expected indentation 0, got 32
> annotations-1.java:5: Expected analysis ((class-close 1)), got ((class-close 21))
>
> ..  So, there are things to sort out before we can apply the change.

Ok, I'll look into them.

> Again, what's the reason for this change?  There must be a test case
> where the current CC Mode handles some Java source code badly.

I use symbol-at-point in java buffers from another mode (javaimp), and @
is just not part of the symbol, but rather, a prefix.  Of course I could
always cut it out, but I think that it should be fixed in cc-mode (as it
may benefit other usages), thus my suggestion.

Filipp

Re: RFR cc-langs.el: change syntax for @ in Java

[Filipp Gunbin]

Hi Alan,

On 08/04/2021 10:15 +0000, Alan Mackenzie wrote:

> I've just run the CC Mode test suite with this amendment, and it
> produces the following successes on annotations-1.java:
>
> annotations-1.java:4: Expected analysis ((inclass 1) (topmost-intro 21)), got ((inclass 21) (topmost-intro 21))
> annotations-1.java:4: Expected indentation 0, got 32
> annotations-1.java:5: Expected analysis ((class-close 1)), got ((class-close 21))
>
> ..  So, there are things to sort out before we can apply the change.

I think these breakages are not real breakages, but rather the resfile
could be updated.

This class:

--8<---------------cut here---------------start------------->8---
@NonNull
@TestClass
@FooBar public class Annotations {

}
--8<---------------cut here---------------end--------------->8---

previosly "started" at the @NonNull line, but now "starts" at @FooBar
line, which is absolutely correct.

So here's the updated patch (now against the cc-mode repo).
Thanks.

diff -r 10abd4587dad cc-langs.el
--- a/cc-langs.el	Mon Mar 29 15:20:53 2021 +0000
+++ b/cc-langs.el	Thu Apr 08 20:22:56 2021 +0300
@@ -365,12 +365,14 @@
        (let ((table (make-syntax-table)))
 	 (c-populate-syntax-table table)
 	 ;; Mode specific syntaxes.
-	 ,(cond ((or (c-major-mode-is 'objc-mode) (c-major-mode-is 'java-mode))
+	 ,(cond ((c-major-mode-is 'objc-mode)
 		 ;; Let '@' be part of symbols in ObjC to cope with
 		 ;; its compiler directives as single keyword tokens.
 		 ;; This is then necessary since it's assumed that
 		 ;; every keyword is a single symbol.
 		 `(modify-syntax-entry ?@ "_" table))
+		((c-major-mode-is 'java-mode)
+		 `(modify-syntax-entry ?@ "'" table))
 		((c-major-mode-is 'pike-mode)
 		 `(modify-syntax-entry ?@ "." table)))
 	 table)))
diff -r 10abd4587dad tests/annotations-1.res
--- a/tests/annotations-1.res	Mon Mar 29 15:20:53 2021 +0000
+++ b/tests/annotations-1.res	Thu Apr 08 20:22:56 2021 +0300
@@ -1,8 +1,8 @@
 ((topmost-intro <0,0>))
 ((annotation-top-cont <1,0>))
 ((annotation-top-cont <1,0>))
-((inclass <3,0>) (topmost-intro <1,0>))
-((class-close <4,0>))
+((inclass <1,0>) (topmost-intro <1,0>))
+((class-close <2,0>))
 ((topmost-intro <1,0>))
 ((topmost-intro <2,0>))
 ((inclass <1,0>) (topmost-intro <1,0>))

Re: RFR cc-langs.el: change syntax for @ in Java

[Alan Mackenzie]

Hello, Filipp.

On Thu, Apr 08, 2021 at 20:26:49 +0300, Filipp Gunbin wrote:
> Hi Alan,

> On 08/04/2021 10:15 +0000, Alan Mackenzie wrote:

> > I've just run the CC Mode test suite with this amendment, and it
> > produces the following successes on annotations-1.java:

> > annotations-1.java:4: Expected analysis ((inclass 1) (topmost-intro 21)), got ((inclass 21) (topmost-intro 21))
> > annotations-1.java:4: Expected indentation 0, got 32
> > annotations-1.java:5: Expected analysis ((class-close 1)), got ((class-close 21))

> > ..  So, there are things to sort out before we can apply the change.

> I think these breakages are not real breakages, but rather the resfile
> could be updated.

I've had a more thorough look at it.  In cc-langs.el, there are several
places where @ is treated as a symbol character, namely:

L375: c-make-mode-syntax-table (which you have proposed modifying)
L419: c-identifier-syntax-modifications
L636: c-symbol-start
L1969: c-paragraph-start (I think this one only applies in comments)
L2198: c-class-decl-kwds (The "@interface" keyword)

We need to consider whether to amend some or all of these places, so
that @ ceases to be a (starting) character of an identifier.

I'm also a bit worried that in

    @NonNull
    @TestClass
    @FooBar
    public class Annotations {

, the last line gets parsed as ((annotation-top-cont 21)), where it
really ought to be a topmost-intro.  But this problem is present without
your patch, anyway.

In summary, I think the motivation for the change (highlighting symbols
correctly) is valid, but the change is going to be more involved than
the patch you've supplied.

Do you want to look at all these things, or should I?


> This class:

> --8<---------------cut here---------------start------------->8---
> @NonNull
> @TestClass
> @FooBar public class Annotations {

> }
> --8<---------------cut here---------------end--------------->8---

> previosly "started" at the @NonNull line, but now "starts" at @FooBar
> line, which is absolutely correct.

Are you saying here that @FooBar is actually a part of the declaration
of class Annotations?  If so, why not @NonNull and @TestClass as well?

> So here's the updated patch (now against the cc-mode repo).
> Thanks.

Thank you!

> diff -r 10abd4587dad cc-langs.el
> --- a/cc-langs.el	Mon Mar 29 15:20:53 2021 +0000
> +++ b/cc-langs.el	Thu Apr 08 20:22:56 2021 +0300
> @@ -365,12 +365,14 @@
>         (let ((table (make-syntax-table)))
>  	 (c-populate-syntax-table table)
>  	 ;; Mode specific syntaxes.
> -	 ,(cond ((or (c-major-mode-is 'objc-mode) (c-major-mode-is 'java-mode))
> +	 ,(cond ((c-major-mode-is 'objc-mode)
>  		 ;; Let '@' be part of symbols in ObjC to cope with
>  		 ;; its compiler directives as single keyword tokens.
>  		 ;; This is then necessary since it's assumed that
>  		 ;; every keyword is a single symbol.
>  		 `(modify-syntax-entry ?@ "_" table))
> +		((c-major-mode-is 'java-mode)
> +		 `(modify-syntax-entry ?@ "'" table))
>  		((c-major-mode-is 'pike-mode)
>  		 `(modify-syntax-entry ?@ "." table)))
>  	 table)))
> diff -r 10abd4587dad tests/annotations-1.res
> --- a/tests/annotations-1.res	Mon Mar 29 15:20:53 2021 +0000
> +++ b/tests/annotations-1.res	Thu Apr 08 20:22:56 2021 +0300
> @@ -1,8 +1,8 @@
>  ((topmost-intro <0,0>))
>  ((annotation-top-cont <1,0>))
>  ((annotation-top-cont <1,0>))
> -((inclass <3,0>) (topmost-intro <1,0>))
> -((class-close <4,0>))
> +((inclass <1,0>) (topmost-intro <1,0>))
> +((class-close <2,0>))
>  ((topmost-intro <1,0>))
>  ((topmost-intro <2,0>))
>  ((inclass <1,0>) (topmost-intro <1,0>))

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: RFR cc-langs.el: change syntax for @ in Java

[Filipp Gunbin]

Hi Alan,

On 12/04/2021 13:36 +0000, Alan Mackenzie wrote:

> I've had a more thorough look at it.  In cc-langs.el, there are several
> places where @ is treated as a symbol character, namely:

I've checked those places before submitting the patch, and I think they
should stay without change.

> L375: c-make-mode-syntax-table (which you have proposed modifying)
> L419: c-identifier-syntax-modifications
> L636: c-symbol-start

I see no problem in leaving them as-is, because they will just continue
to match @ with the following chars as a whole.  It's just when you need
more fine-grained distinction, then you could check if it's prefix or
symbol.

Prefix syntax is defined so that prefix chars are part of the following
expression, that's exactly the case of Java annotations.

> L1969: c-paragraph-start (I think this one only applies in comments)

Yes, this is for javadoc tags, which start with @, and appear only in
comments, like:

/**
 * @param my-param ..
 */

> L2198: c-class-decl-kwds (The "@interface" keyword)

@interface is a keyword which declares the annotation interface itself.

I would really hope that syntax tables do not affect parsing/matching of
keywords.  If they don't, then this place doesn't need to be changed,
either.

> We need to consider whether to amend some or all of these places, so
> that @ ceases to be a (starting) character of an identifier.

I don't think it should cease to be.

> I'm also a bit worried that in
>
>     @NonNull
>     @TestClass
>     @FooBar
>     public class Annotations {
>
> , the last line gets parsed as ((annotation-top-cont 21)), where it
> really ought to be a topmost-intro.  But this problem is present without
> your patch, anyway.
>
> In summary, I think the motivation for the change (highlighting symbols
> correctly) is valid, but the change is going to be more involved than
> the patch you've supplied.

It's not about highlighting, but rather about "extracting" @, to be able
to request the non-@ part separately.

But maybe I'm missing something.

Thanks.
Filipp

Re: RFR cc-langs.el: change syntax for @ in Java

[Alan Mackenzie]

Hello, Filipp.

On Mon, Apr 12, 2021 at 20:33:39 +0300, Filipp Gunbin wrote:
> Hi Alan,

> On 12/04/2021 13:36 +0000, Alan Mackenzie wrote:

> > I've had a more thorough look at it.  In cc-langs.el, there are several
> > places where @ is treated as a symbol character, namely:

> I've checked those places before submitting the patch, and I think they
> should stay without change.

OK, thanks!

> > L375: c-make-mode-syntax-table (which you have proposed modifying)
> > L419: c-identifier-syntax-modifications
> > L636: c-symbol-start

> I see no problem in leaving them as-is, because they will just continue
> to match @ with the following chars as a whole.  It's just when you need
> more fine-grained distinction, then you could check if it's prefix or
> symbol.

> Prefix syntax is defined so that prefix chars are part of the following
> expression, that's exactly the case of Java annotations.

Prefix syntax hasn't been used in CC Mode up till now, so it will be
something of an experiment.

> > L1969: c-paragraph-start (I think this one only applies in comments)

> Yes, this is for javadoc tags, which start with @, and appear only in
> comments, like:

> /**
>  * @param my-param ..
>  */

> > L2198: c-class-decl-kwds (The "@interface" keyword)

> @interface is a keyword which declares the annotation interface itself.

> I would really hope that syntax tables do not affect parsing/matching of
> keywords.  If they don't, then this place doesn't need to be changed,
> either.

> > We need to consider whether to amend some or all of these places, so
> > that @ ceases to be a (starting) character of an identifier.

> I don't think it should cease to be.

OK.

> > I'm also a bit worried that in

> >     @NonNull
> >     @TestClass
> >     @FooBar
> >     public class Annotations {

> > , the last line gets parsed as ((annotation-top-cont 21)), where it
> > really ought to be a topmost-intro.  But this problem is present without
> > your patch, anyway.

> > In summary, I think the motivation for the change (highlighting symbols
> > correctly) is valid, but the change is going to be more involved than
> > the patch you've supplied.

This is, I'm sure, a separate problem.

> It's not about highlighting, but rather about "extracting" @, to be able
> to request the non-@ part separately.

> But maybe I'm missing something.

You've just told me you've looked into things more thoroughly than you
told me before.  Thanks!

I now agree with you that we should commit this change, both to CC Mode
and to Emacs.  Do you want me to do this (with your name), or would you
prefer to do it yourself (at least in Emacs)?

> Thanks.
> Filipp

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: RFR cc-langs.el: change syntax for @ in Java

[Filipp Gunbin]

Hi Alan,

On 13/04/2021 10:28 +0000, Alan Mackenzie wrote:

> Prefix syntax hasn't been used in CC Mode up till now, so it will be
> something of an experiment.

Yes, I saw that…  Maybe that'll uncover problems which will require
reverting or fixing, let's see.

> I now agree with you that we should commit this change, both to CC Mode
> and to Emacs.  Do you want me to do this (with your name), or would you
> prefer to do it yourself (at least in Emacs)?

What is more convenient to you?  I can commit into Emacs.  For
standalone cc-mode, I don't have commit rights.  If you regularly sync
Emacs code with upstream cc-mode (though a quick look at Emacs commit
history doesn't show me such merges), then maybe it's more convenient to
merge into cc-mode, and then it will arrive in Emacs some time later.

Thanks.

Re: RFR cc-langs.el: change syntax for @ in Java

[Alan Mackenzie]

Hello, Filipp.

On Tue, Apr 13, 2021 at 17:45:08 +0300, Filipp Gunbin wrote:
> Hi Alan,

> On 13/04/2021 10:28 +0000, Alan Mackenzie wrote:

> > Prefix syntax hasn't been used in CC Mode up till now, so it will be
> > something of an experiment.

> Yes, I saw that…  Maybe that'll uncover problems which will require
> reverting or fixing, let's see.

Indeed, we will see (or not).

> > I now agree with you that we should commit this change, both to CC Mode
> > and to Emacs.  Do you want me to do this (with your name), or would you
> > prefer to do it yourself (at least in Emacs)?

> What is more convenient to you?

I've just committed the change (with your name) to all the appropriate
places.

> I can commit into Emacs.  For standalone cc-mode, I don't have commit
> rights.  If you regularly sync Emacs code with upstream cc-mode
> (though a quick look at Emacs commit history doesn't show me such
> merges), then maybe it's more convenient to merge into cc-mode, and
> then it will arrive in Emacs some time later.

CC Mode runs under Mercurial (a better VCS than git ;-).   So I'm not
sure an upstream merge operation would be possible.  I just commit to
both places separately.

> Thanks.

Thanks.  I think we can now regard the change as done and closed.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: RFR cc-langs.el: change syntax for @ in Java

[Ergus]

>CC Mode runs under Mercurial (a better VCS than git ;-).   

There are people that just want to see the blood run... :p... what's next? vim is better or windows is better? :)))

-- 

Re: RFR cc-langs.el: change syntax for @ in Java

[Filipp Gunbin]

On 14/04/2021 19:55 +0000, Alan Mackenzie wrote:

> I've just committed the change (with your name) to all the appropriate
> places.

Great, thank you!