Output column info from compiler error when available

[cmaglie]

The output was missing column info:

before the patch:

/home/cmaglie/Code/arduino/build/linux/work/examples/01.Basics/Blink/Blink.ino: In function 'void setup()':
Blink:29: error: 'xxxx' was not declared in this scope
   xxxx
   ^
exit status 1

after the patch:

/home/cmaglie/Code/arduino/build/linux/work/examples/01.Basics/Blink/Blink.ino: In function 'void setup()':
Blink:29:1: error: 'xxxx' was not declared in this scope
 xxxx
 ^
exit status 1

[ArduinoBot]

✅ Build completed.

Please test this code using one of the following:

⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6897-BUILD-735-linux32.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6897-BUILD-735-linux64.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6897-BUILD-735-windows.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6897-BUILD-735-macosx.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6897-BUILD-735-linuxarm.tar.xz

ℹ️ The linuxarm build is still experimental and may not be always available.

[matthijskooijman]

I was wondering: Why do we even re-construct the error message in the first place, and not just leave "s" untouched, but I guess this is because we want to translate the filename (from the temporary copy to the original filename). Perhaps it's good to separate these two cases, though:

• Changing the filename
• Splitting the error to figure out line (and column) info

Or perhaps keep a single regex, but provide an additional broad capture for "everything but the filename", e.g.:

"(.+\\.\\w+):((\\d+)(:\\d+)*:\\s*error:\\s*(.*)\\s*)"

Now, you can munch the filename in pieces[1], append pieces[2] and be sure that you have the original error message as much as possible (without having to reassemble it from multiple pieces, hardcoding "error" in the process).

Another thing I'm wondering, what if there's multiple column numbers (should never happen I think, but the regex allows it by using (:\\d+)*). That would probably break the parseInt() (if all matches are returned), or drop data (if only the first or last one is returned). I guess changing the regex to (:\\d+)? would be sane?

One completely different approach for munching the filename would be to insert #line directives in the copied files so the generated error message are correct from the start. But I'm also still hoping to get better support for compiling files directly (without making a copy in the build dir), which would also heavily improve this situation, but I just haven't gotten around to this yet...

[cmaglie]

Or perhaps keep a single regex, but provide an additional broad capture for "everything but the filename"

At the moment the error "reconstruction" is a bit ugly, I agree, but it works and it's tested I would not change it.

Another thing I'm wondering, what if there's multiple column numbers (should never happen I think, but the regex allows it by using (:\d+)*). That would probably break the parseInt() (if all matches are returned), or drop data (if only the first or last one is returned). I guess changing the regex to (:\d+)? would be sane?

In this case the extra pieces[] are ignored:

      String msg = "";
      int errorIdx = pieces.length - 1;
      String error = pieces[errorIdx];
      String filename = pieces[1];
      int line = PApplet.parseInt(pieces[2]);
      int col;
      if (errorIdx > 3) {
        col = PApplet.parseInt(pieces[3].substring(1));
      } else {
        col = -1;
      }

error is always the last element, then follows filename and line, and column is filled only if there are more than 3 elements. Is also worth noting that if those (hypothetical) extra pieces are present they will not be printed in the output. Changing the regexp to (:\\d+) will make the regexp to not match the error if the extra pieces are present, again I don't know if it worth changing it now...

[matthijskooijman]

Oh, you mean that (:\\d+)* results in (potentially) multiple elements in pieces? I hadn't realized that possibility (often this does not happen, because it breaks the usefulness of hardcoded backreferences, e.g. $1 etc.).

But indeed, it's not really a big deal, just leaving it as is is fine by me as well.

Looking at the diff again (and ignoring existing ugliness :-p), it looks good to me.