Hi All,
Two ideas for the Compiler tab open for consideration:
1) Since both C/C++ compilers (g++, clang++) now output little ^ characters which are supposed to point to the place where they got confused, I suggest that the compiler tab use a (real) monospace font by default, so the ^ is in the intended place.
2) After a compile, the tab should be scrolled back to the top since, AFAIK, most people clear up reported errors from the first to the last so they can identify and skip consequential errors caused by a prior compile error.
Cheers Lex
Am 11.01.2014 10:35, schrieb Lex Trotman:
Hi All,
Two ideas for the Compiler tab open for consideration:
- Since both C/C++ compilers (g++, clang++) now output little ^
characters which are supposed to point to the place where they got confused, I suggest that the compiler tab use a (real) monospace font by default, so the ^ is in the intended place.
Not sure is the ^ really useful to us in geany? I'm not sure because clicking on the line moves the editor to that location anyway.
- After a compile, the tab should be scrolled back to the top since,
AFAIK, most people clear up reported errors from the first to the last so they can identify and skip consequential errors caused by a prior compile error.
No, not to the top. If anything to the first error, but staying at the bottom is just fine. Compiler errors usually happen at the end so if you scroll to the top you might have to scroll all the way down again. This might be a lot to scroll for large projects where hundreds of files compiled successfully before the first error. FWIW, I usually fix compile errors in no specific order (usually I fix the first line, upwards from the bottom, where I can distinguish an error from a warning).
Best regards.
On 11 January 2014 21:00, Thomas Martitz kugel@rockbox.org wrote:
Am 11.01.2014 10:35, schrieb Lex Trotman:
Hi All,
Two ideas for the Compiler tab open for consideration:
- Since both C/C++ compilers (g++, clang++) now output little ^
characters which are supposed to point to the place where they got confused, I suggest that the compiler tab use a (real) monospace font by default, so the ^ is in the intended place.
Not sure is the ^ really useful to us in geany? I'm not sure because clicking on the line moves the editor to that location anyway.
Yes clicking goes to the line, but doesn't indicate the column, which is what ^ does.
It would be better if it *did* go to the column but that needs focus forced to the editing widget to show the new cursor position. Focussing by clicking unfortunately makes the cursor go where you click :)
I just (belatedly) checked and there is PR #191 and patch #11 both purporting to handle column numbers, not sure of their status.
#191 is quite big but on quick glance I didn't actually see where the column was handled, and sf is so slow I ran out of patience looking for the patch 11.
- After a compile, the tab should be scrolled back to the top since,
AFAIK, most people clear up reported errors from the first to the last so they can identify and skip consequential errors caused by a prior compile error.
No, not to the top. If anything to the first error, but staying at the bottom is just fine. Compiler errors usually happen at the end so if you scroll to the top you might have to scroll all the way down again. This might be a lot to scroll for large projects where hundreds of files compiled successfully before the first error. FWIW, I usually fix compile errors in no specific order (usually I fix the first line, upwards from the bottom, where I can distinguish an error from a warning).
Yes agree, first error would be best.
Cheers Lex
Best regards. _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On Sat, 11 Jan 2014 21:28:42 +1100 Lex Trotman elextr@gmail.com wrote:
On 11 January 2014 21:00, Thomas Martitz kugel@rockbox.org wrote:
Not sure is the ^ really useful to us in geany? I'm not sure because clicking on the line moves the editor to that location anyway.
Yes clicking goes to the line, but doesn't indicate the column, which is what ^ does.
It would be better if it *did* go to the column but that needs focus forced to the editing widget to show the new cursor position. Focussing by clicking unfortunately makes the cursor go where you click :)
I just (belatedly) checked and there is PR #191 and patch #11 both purporting to handle column numbers, not sure of their status.
Patch 11 works without changes since Mar-2012. It handles the buildin parsing, and explains in geany.txt the optional 3rd regex for column. Generally, it's a simple extension to the current parsing mechanism.
#191 is quite big but on quick glance I didn't actually see where the column was handled, and sf is so slow I ran out of patience looking for the patch 11.
I attached a fresh copy of patch 11 which applies without offsets.
On 11/01/2014 12:28, Lex Trotman wrote:
Yes clicking goes to the line, but doesn't indicate the column, which is what ^ does.
It would be better if it *did* go to the column but that needs focus forced to the editing widget to show the new cursor position. Focussing by clicking unfortunately makes the cursor go where you click :)
I just (belatedly) checked and there is PR #191 and patch #11 both purporting to handle column numbers, not sure of their status.
#191 is quite big but on quick glance I didn't actually see where the column was handled, and sf is so slow I ran out of patience looking for the patch 11.
PR 191 is pretty much good to go. I might have missed a thing or two in the makefiles. The column numbers are handled once when the error message is parsed, and again when actually jumping to that location. The rest of the code is agnostic of column numbers and many other details, since they're all encapsulated in the new GeanyFileLocation type (which, while a little big, is mostly boilerplate). This allows us to easily deal with different kinds of "column numbers" that different tools produce, be they character numbers, actual column numbers or byte offsets of some kind.
I took a quick glance at patch 11 (which is older than mine and I was unaware of. I didn't mean to be a dick, Dimitar :) It's not as comprehensive and a bit problematic. It treats compiler "column numbers" as if they're actually column numbers, while in reality they almost never are. Which means that it's not going to work right as soon as the code contains tabs or multibyte characters. Since it doesn't use named capture groups for the message parsing, it's a little less flexible in the type of messages it can parse, and it also cannot handle different styles of error messages in the same regex. The example given in the documentation is actually not going to work when the column number is missing.
On Sat, 18 Jan 2014 17:10:37 +0200 Arthur Rosenstein artros.misc@gmail.com wrote:
I took a quick glance at patch 11 [...] It treats compiler "column numbers" as if they're actually column numbers, while in reality they almost never are. Which means that it's not going to work right as soon as the code contains tabs or multibyte characters.
Hmmm... :)
<tab><tab>(s - text == 10 && "боза" xxx ... [utf-8]
utils.c:643:33: error: expected ')' before 'xxx'
Double click on the message: seeks to the first 'x', Geany status bar shows line 643, column 43 (column # depends on the tab size).
But it *does* seek to 'а' if "боза" is locale.
Since it doesn't use named capture groups for the message parsing, it's a little less flexible in the type of messages it can parse
line [column] filename, or filename line [column]
If no regex is specified (and most people leave it blank I think), the "fallback" Geany parser requires only one of filename or line (default filename == that of the current document).
With a regex, both the filename and line are mandatory (though the regex Geany parser can be modified to make the line optional).
P11 adds an optional column without altering the above behaviour.
and it also cannot handle different styles of error messages in the same regex.
expr1|expr2|...
Or if you mean errors vs. warnings, Geany does not support such a distinction, and P11 only adds an optional column.
The example given in the documentation is actually not going to work when the column number is missing.
echo "test.py:7:24: E202 whitespace before ']'" | egrep '([^:]+):([0-9] +):([0-9]+): |([^:]+):([0-9]+): ' test.py:7:24: E202 whitespace before ']'
echo "test.py:7: E202 whitespace before ']'" | egrep '([^:]+):([0-9]+): ([0-9]+): |([^:]+):([0-9]+): ' test.py:7: E202 whitespace before ']'
Overall, P11 is a minimal implementation, which handles most of the cases, with very small regex changes (an optional 3rd match). PR 191 is the maximum implementation. Both have pros and cons.
IMHO, such questions should be decided with a vote.
On 19/01/2014 14:51, Dimitar Zhekov wrote:
On Sat, 18 Jan 2014 17:10:37 +0200 Arthur Rosenstein artros.misc@gmail.com wrote:
I took a quick glance at patch 11 [...] It treats compiler "column numbers" as if they're actually column numbers, while in reality they almost never are. Which means that it's not going to work right as soon as the code contains tabs or multibyte characters.
Hmmm... :)
<tab><tab>(s - text == 10 && "боза" xxx ... [utf-8]
utils.c:643:33: error: expected ')' before 'xxx'
Double click on the message: seeks to the first 'x', Geany status bar shows line 643, column 43 (column # depends on the tab size).
But it *does* seek to 'а' if "боза" is locale.
Indeed, my bad. You're not treating "column numbers" as column numbers but rather UTF-8 byte offsets. That's definitely more common, but not universal still.
Since it doesn't use named capture groups for the message parsing, it's a little less flexible in the type of messages it can parse
line [column] filename, or filename line [column]
If no regex is specified (and most people leave it blank I think), the "fallback" Geany parser requires only one of filename or line (default filename == that of the current document).
With a regex, both the filename and line are mandatory (though the regex Geany parser can be modified to make the line optional).
P11 adds an optional column without altering the above behaviour.
I think it's a better idea to reduce the surface area by moving everything to the regex based parser and getting rid of the hard-coded one. I left it as a fallback just to be on the safe side, but if regex based parsing proves to work well for a release or two it can be safely dropped.
and it also cannot handle different styles of error messages in the same regex.
expr1|expr2|...
Won't work. See blow.
Or if you mean errors vs. warnings, Geany does not support such a distinction, and P11 only adds an optional column.
That's not what I mean.
The example given in the documentation is actually not going to work when the column number is missing.
echo "test.py:7:24: E202 whitespace before ']'" | egrep '([^:]+):([0-9] +):([0-9]+): |([^:]+):([0-9]+): ' test.py:7:24: E202 whitespace before ']'
echo "test.py:7: E202 whitespace before ']'" | egrep '([^:]+):([0-9]+): ([0-9]+): |([^:]+):([0-9]+): ' test.py:7: E202 whitespace before ']'
Did you actually test it? You're using g_match_info_fetch() to read the submatches of the first three *capture groups*. Those are indexed independently of the string you're trying to match. So, even though the pattern does match the error message in the file:line case, you're reading the source-location info from the capture groups that didn't match anything. Note that in this case the solution is simple enough: "([^:]+):([0-9]+)(?::([0-9]+))?: ", but the general limitation is still there.
Overall, P11 is a minimal implementation, which handles most of the cases, with very small regex changes (an optional 3rd match). PR 191 is the maximum implementation. Both have pros and cons.
I think this is a slight overstatement. PR191 is really not *that* much bigger or radical, and a lot of the code that it adds is modular and can be used for other things (which I do, FWIW, in the Compiler Errors window for example).
On Sun, 19 Jan 2014 19:34:01 +0200 Arthur Rosenstein artros.misc@gmail.com wrote:
utils.c:643:33: error: expected ')' before 'xxx'
Double click on the message: seeks to the first 'x', Geany status bar shows line 643, column 43 (column # depends on the tab size).
But it *does* seek to 'а' if "боза" is locale.
Indeed, my bad. You're not treating "column numbers" as column numbers but rather UTF-8 byte offsets. That's definitely more common, but not universal still.
A truly universal solution should take into account whether the compiler emits utf-8 columns (java?/net?/python3?), or what is the file locale + whether the compiler takes into account the system locale + what is the system locale. Geany sets the GLib I/O channel encoding to NULL "safe to use with binary data", so at least that should not have any effect.
echo "test.py:7: E202 whitespace before ']'" | egrep '([^:]+):([0-9]+):([0-9]+):|([^:]+):([0-9]+): ' test.py:7: E202 whitespace before ']'
Did you actually test it? You're using g_match_info_fetch() to read the submatches of the first three *capture groups*. Those are indexed independently of the string you're trying to match. So, even though the pattern does match the error message in the file:line case, you're reading the source-location info from the capture groups that didn't match anything.
No, I assumed that Geany works as documented, and reads the first two matches. Looking at regex(1), it probably worked like that before switching to GRegex.
Note that in this case the solution is simple enough: "([^:]+):([0-9]+)(?::([0-9]+))?: ", but the general limitation is still there.
It's not a limitation, but a bug in the current regex parser. We should either document that the first two capturing groups are used, no matter if they match, or skip the non-matching groups (which is quite easy). After that, P11 can be updated - fixing the regex parser is outside it's scope.
[...]
It's not a limitation, but a bug in the current regex parser. We should either document that the first two capturing groups are used, no matter if they match, or skip the non-matching groups (which is quite easy). After that, P11 can be updated - fixing the regex parser is outside it's scope.
Yes, there was a misunderstanding (forget who, but remember mentioning it) that the capture numbers of Gregex are static, match one is the first capture () match two the second () even if they did not match. Seems like the code wasn't fixed though. Filetypes_parse_error_message() just needs to loop through the matches until it finds the first two that actually matched.
Cheers Lex
-- E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Anyhow ... returning to the initial proposal :)
In respect of item 1) this is all I'm proposing:
diff --git a/src/keyfile.c b/src/keyfile.c index 102fb78..bdf7a0a 100644 --- a/src/keyfile.c +++ b/src/keyfile.c @@ -78,7 +78,7 @@ #define GEANY_DEFAULT_TOOLS_GREP "grep" #define GEANY_DEFAULT_MRU_LENGTH 10 #define GEANY_DEFAULT_FONT_SYMBOL_LIST "Sans 9" -#define GEANY_DEFAULT_FONT_MSG_WINDOW "Sans 9" +#define GEANY_DEFAULT_FONT_MSG_WINDOW "Monospace 9" #define GEANY_DEFAULT_FONT_EDITOR "Monospace 10" #define GEANY_TOGGLE_MARK "~ " #define GEANY_MAX_AUTOCOMPLETE_WORDS 30
ie change the ***DEFAULT*** font for new configs. Existing configs will not change. The user can change it in the GUI at menu->edit_preferences->interface->interface.
Cheers Lex
In respect of suggestion 2), the various pref compiler_tab_autoscroll provides the equivalent of returning to the top by preventing the tab ever leaving the top :) Thats enough for me, suggestion 2) withdrawn :)
Cheers Lex
On 11/01/2014 11:35, Lex Trotman wrote:
Hi All,
Two ideas for the Compiler tab open for consideration:
- Since both C/C++ compilers (g++, clang++) now output little ^
characters which are supposed to point to the place where they got confused, I suggest that the compiler tab use a (real) monospace font by default, so the ^ is in the intended place.
I agree. Raw output is displayed in fixed-width font almost by convention.
- After a compile, the tab should be scrolled back to the top since,
AFAIK, most people clear up reported errors from the first to the last so they can identify and skip consequential errors caused by a prior compile error.
Probably relevant: I'm working on a Compiler Errors window (screenshot [1]) which displays errors and other messages in a more organized fashion. It's similar to the Errors List window in Visual Studio or Problems view in Eclipse. It allows the user to filter, sort and keep track of compilation issues. I can give more details if there's interest in this.
[1] https://raw2.github.com/artros/images/master/geany/compiler-errors-window.pn...