Re: Review request: javac in compilation-error-regexp-alist-alist

[Filipp Gunbin]

Hi Mattias, thanks for the helpful comments, corrected patch below.

On 28/02/2020 18:11 +0100, Mattias Engdegård wrote:

> 27 feb. 2020 kl. 19.17 skrev Filipp Gunbin <address@hidden>:
>
>> Besides that, `java' symbol in the said list is a misnomer: it handles
>> java exceptions (why? we would never normally have them in the
>> compilation output), and valgrind output.  Looks like we should just
>> rename it to `valgrind'.
>
> Maybe, but as you can see there are several questionably-named entries
> in the list. Renaming them may break user customisation.

Good point.

> As it is, it would be useful with a comment explaining the difference
> between 'java' and 'javac' in the list.

One mentions java exceptions, another - javac compiler, I guess that's
enough.

> (Could I entice you into writing this regexp in rx? Not mandatory in
> any way, but at least some of us who read it will thank you.)

I'm not experienced with it yet, but noted, will do later.

> There's an unnecessary ambiguity after the line number and colon; a
> string of spaces can match in multiple ways. Also, avoid [[:space:]]
> since you don't control that syntax class in the buffer; more likely
> it should just be plain spaces, as it's very unlikely that the javac
> caret output routine would use anything else.

All corrected.

>> +     1 2
>> +     (lambda ()
>
> If you add a comma before the lambda-expression, it gets evaluated,
> byte-compiled, syntax-checked etc.

Right.

>> +    ("/src/Test.java:5: ';' expected\n        foo foo\n               ^\n" 
>> 1 15 5 "/src/Test.java" 2)
>> +    ("e:\\src\\Test.java: 7: warning: ';' expected\n   foo foo\n          
>> ^\n" 1 10 7 "e:\\src\\Test.java" 1)
>
> Why the differences in spacing before the line number? Is it a
> difference between platforms, error/warning, javac versions, or just
> different compilers altogether? Comments explaining this would be
> useful. Be sure to include variants in compilation.txt.

Completely imagined by me, removed.

Filipp


diff --git a/etc/compilation.txt b/etc/compilation.txt
index ebce6a14d0..ed2eecd778 100644
--- a/etc/compilation.txt
+++ b/etc/compilation.txt
@@ -237,6 +237,19 @@ Register 6 contains wrong type
 ==1332==    by 0x8008621: main (vtest.c:180)
 
 
+* javac Java compiler
+
+symbol: javac
+
+Should also work when compiling Java with Gradle.  We don't have
+column number, but we try to deduce it from position of "^".
+
+Test.java:5: error: ';' expected
+        foo foo
+               ^
+1 error
+
+
 * IBM jikes
 
 symbols: jikes-file jikes-line
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 455f181f50..6411c577f7 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -265,6 +265,23 @@ compilation-error-regexp-alist-alist
     (java
      "^\\(?:[ \t]+at \\|==[0-9]+== 
+\\(?:at\\|b\\(y\\)\\)\\).+(\\([^()\n]+\\):\\([0-9]+\\))$" 2 3 nil (1))
 
+    (javac
+     ,(concat
+       ;; line1
+       "^\\(\\(?:[A-Za-z]:\\)?[^:\n]+\\):" ;file
+       "\\([0-9]+\\): "                    ;line
+       "\\(?:\\(?:error\\|\\(warning\\)\\): \\)?[^\n]*\n" ;type (optional) and 
message
+       ;; line2: source line containing error
+       "[^\n]*\n"
+       ;; line3: single "^" under error position in line2
+       " *\\^\n")
+     1 2
+     ,(lambda ()
+        (save-excursion
+          (backward-char 2)             ; move back over "^\n"
+          (current-column)))
+     (3))
+
     (jikes-file
      "^\\(?:Found\\|Issued\\) .* compiling \"\\(.+\\)\":$" 1 nil nil 0)
 
@@ -1455,9 +1472,15 @@ compilation-parse-errors
         nil) ;; Not anchored or anchored but already allows empty spaces.
        (t (setq pat (concat "^\\(?:      \\)?" (substring pat 1)))))
 
-      (if (consp file) (setq fmt (cdr file)      file (car file)))
-      (if (consp line) (setq end-line (cdr line) line (car line)))
-      (if (consp col)  (setq end-col (cdr col)   col (car col)))
+      (if (and (consp file) (not (functionp file)))
+         (setq fmt (cdr file)
+                file (car file)))
+      (if (and (consp line) (not (functionp line)))
+          (setq end-line (cdr line)
+                line (car line)))
+      (if (and (consp col) (not (functionp col)))
+          (setq end-col (cdr col)
+                col (car col)))
 
       (unless (or (null (nth 5 item)) (integerp (nth 5 item)))
         (error "HYPERLINK should be an integer: %s" (nth 5 item)))
diff --git a/test/lisp/progmodes/compile-tests.el 
b/test/lisp/progmodes/compile-tests.el
index 75962566f1..cd736497e6 100644
--- a/test/lisp/progmodes/compile-tests.el
+++ b/test/lisp/progmodes/compile-tests.el
@@ -176,6 +176,9 @@ compile-tests--test-regexps-data
      13 nil 217 "../src/Lib/System.cpp")
     ("==1332==    by 0x8008621: main (vtest.c:180)"
      13 nil 180 "vtest.c")
+    ;; javac
+    ("/src/Test.java:5: ';' expected\n        foo foo\n               ^\n" 1 
15 5 "/src/Test.java" 2)
+    ("e:\\src\\Test.java:7: warning: ';' expected\n   foo foo\n          ^\n" 
1 10 7 "e:\\src\\Test.java" 1)
     ;; jikes-file jikes-line
     ("Found 2 semantic errors compiling \"../javax/swing/BorderFactory.java\":"
      1 nil nil "../javax/swing/BorderFactory.java")
@@ -431,8 +434,8 @@ compile-test-error-regexps
           (compilation-num-warnings-found 0)
           (compilation-num-infos-found 0))
       (mapc #'compile--test-error-line compile-tests--test-regexps-data)
-      (should (eq compilation-num-errors-found 93))
-      (should (eq compilation-num-warnings-found 36))
+      (should (eq compilation-num-errors-found 94))
+      (should (eq compilation-num-warnings-found 37))
       (should (eq compilation-num-infos-found 26)))))
 
 (ert-deftest compile-test-grep-regexps ()