Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change syntax errors/warnings to use FILE:LINE format #469

Merged
merged 2 commits into from
Feb 18, 2016

Conversation

fingolfin
Copy link
Member

This changes syntax errors and warnings to print slightly differently. Before:

gap> Read("test.g");
Syntax error: expression expected in test.g line 1
*1;
^

After:

gap> Read("test.g");
Syntax error: expression expected in test.g:1
*1;
^
gap>

The primary reason I'd like to see that is selfish: Such strings are recognized by my terminal, and I can Cmd-click them to open an editor for that file, at the correct line.

This format also matches the output of gcc and clang when it comes to errors:

~/Projekte/GAP/gap.github (git:mh/syntax-error-lines)$ clang test.c
test.c:1:2: error: expected identifier or '('
*1;
 ^
1 error generated.
~/Projekte/GAP/gap.github (git:mh/syntax-error-lines)$ gcc-5 test.c
test.c:1:2: error: expected identifier or ‘(’ before numeric constant
 *1;
  ^

I realize that might be a somewhat bizarre reason, but I thought I could at least try to convince you this is useful ;-).

Of course, there is a the potential drawback that end users may not understand the new output.

@olexandr-konovalov
Copy link
Member

+1

@fingolfin fingolfin added the gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 label Jan 15, 2016
@@ -329,8 +329,12 @@ InstallGlobalFunction("Test", function(arg)
if d[1] <> '-' then
d := Concatenation("+", d);
fi;
Print("########> Time diff in ",
fnam,", line ",line,":\n");
Print("########> Time diff in ")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No semicolon in this line - that's what causing Travis tests to fail. Please fix it and I suggest to merge it - no objections expressed during three weeks.

@markuspf
Copy link
Member

I'd merge this, but tests are failing :/

@olexandr-konovalov
Copy link
Member

@markuspf someone just to check systematically and update error messages included in tests, eg

Syntax error: Only two dots in a range in stream line 1

should be

Syntax error: Only two dots in a range in stream:1

@markuspf
Copy link
Member

On Thu, Feb 18, 2016 at 07:19:46AM -0800, Alexander Konovalov wrote:

@markuspf someone just to check systematically and update error messages
included in the test, eg

The operative words being "someone" and "just".

@markuspf markuspf merged commit 17ca2a9 into gap-system:master Feb 18, 2016
@fingolfin
Copy link
Member Author

I just saw this message, and wanted to see what errors there are, and look into resolving them. But I see this has now been merged anyway? I am confused?!

@fingolfin
Copy link
Member Author

Aha, it seems @markuspf took care of that in 96c590a -- thank you!

@markuspf
Copy link
Member

On Thu, Feb 18, 2016 at 10:25:36AM -0800, Max Horn wrote:

Aha, it seems @markuspf took care of that in 96c590a -- thank you!

Yeah, I wouldn't merge this with the errors still present, and figured that a
quick sed is better than this discussion going on further ;)

@fingolfin fingolfin deleted the mh/syntax-error-lines branch August 18, 2016 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants