Implement issue 20515: add command line flag to allow selecting GNU message style for errors and warnings. #10732

FeepingCreature · 2020-01-17T10:50:04Z

This should ease interoperability with existing IDEs.

The GNU style is documented at https://www.gnu.org/prep/standards/html_node/Errors.html

dlang-bot · 2020-01-17T10:50:08Z

Thanks for your pull request and interest in making D better, @FeepingCreature! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✗	20515	enhancement	Errors should be reported in GNU style on Linux

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10732"

src/dmd/globals.d

src/dmd/mars.d

FeepingCreature · 2020-01-17T11:42:11Z

sorry for the spam, c++ interop wouldn't go

wilzbach

You might even want to add -verrors=context to this new flag as it's similarly just about the formatting style.

wilzbach · 2020-01-17T12:08:25Z

src/dmd/cli.d

@@ -641,6 +641,12 @@ dmd -cov -unittest myprog.d
        Option("vcolumns",
            "print character (column) numbers in diagnostics"
        ),
+        Option("verror-style=digitalmars",


Suggested change

Option("verror-style=digitalmars",

Option("verror-style=[digitalmars|gnu|?]",

See the other options.

better? Not sure '?' is worth it here, the options are fairly self-explanatory.

Most DMD options have a help page that lists all available options.

the options are fairly self-explanatory.

Fair enough.

src/dmd/globals.d

FeepingCreature · 2020-01-17T12:23:06Z

@wilzbach not sure what you mean with "add -verrors=context"

wilzbach · 2020-01-17T12:32:13Z

@wilzbach not sure what you mean with "add -verrors=context"

It's an existing output error style:

> dmd -verrors=context foo.d
foo.d(1): Error: declaration expected, not =
a = b
  ^

It would fit more with the new flag.

FeepingCreature · 2020-01-17T13:23:47Z

I get what it is, I don't know what you mean by "fit more with the new flag".

edit: Do you mean -verrors=gnu?

FeepingCreature · 2020-01-31T07:56:42Z

ping @wilzbach

Geod24

Can we have a changelog entry ? Many people probably want to add that to their dmd.conf. I know I do.

Geod24 · 2020-01-31T10:05:57Z

src/dmd/mars.d

+            else if (strcmp(style, "gnu") == 0)
+                params.messageStyle = MessageStyle.gnu;
+            else
+                error("unknown error style '%s'", style);


List possible style

src/dmd/globals.d

…essage style for errors and warnings.

FeepingCreature · 2020-01-31T10:29:24Z

Added a changelog entry.

FeepingCreature · 2020-01-31T11:16:51Z

@Geod24 thanks for automerge but could you set it to 72h instead? I would still like to understand what @wilzbach meant.

thewilsonator · 2020-01-31T11:18:21Z

Done. (N.B 72h merge stoped sef-merging a while ago)

wilzbach · 2020-01-31T11:39:56Z

I get what it is, I don't know what you mean by "fit more with the new flag".

The one that you introduce.
So -verrors=context -> -verror-style=context as it's an error style as well. WDYT?

FeepingCreature · 2020-01-31T13:45:20Z

Ah, I finally understand. I'm not sure that's a good idea, as it seems to do a very different thing? So we'd need to do it "feature flag" style, ie something like -verror-style=context,gnu which seems a bit awkward, or -verror-style=context -verror-style=gnu which looks like we're overriding context.

wilzbach · 2020-01-31T14:16:08Z

Ah, I finally understand.

Sorry about that.

N.B 72h merge stoped sef-merging a while ago)

It never did that. A merge should always be a conscious decision. You can get a list of all of them easily over all repos e.g. via https://github.com/pulls?utf8=%E2%9C%93&q=is%3Aopen+is%3Apr+org%3Adlang+label%3A%2272h+no+objection+-%3E+merge%22

I'm not sure that's a good idea, as it seems to do a very different thing?

IMHO it's more an -verror-style than an -verrors option.

So we'd need to do it "feature flag" style

Yeah we would, what do other people think about this?

MoonlightSentinel · 2020-01-31T14:58:13Z

Yeah we would, what do other people think about this?

I don't think this is a good idea because feature flags should IMHO accept any subset of all possible values. That would imply -verror-style=digitalmars,gnu as a valid configuration allhough they are mutually exclusive.

FeepingCreature requested review from Geod24, MartinNowak, rainers and WalterBright as code owners January 17, 2020 10:50

thewilsonator reviewed Jan 17, 2020

View reviewed changes

src/dmd/globals.d Outdated Show resolved Hide resolved

thewilsonator reviewed Jan 17, 2020

View reviewed changes

src/dmd/mars.d Outdated Show resolved Hide resolved

CyberShadow reviewed Jan 17, 2020

View reviewed changes

src/dmd/mars.d Outdated Show resolved Hide resolved

src/dmd/mars.d Outdated Show resolved Hide resolved

src/dmd/mars.d Outdated Show resolved Hide resolved

src/dmd/mars.d Outdated Show resolved Hide resolved

FeepingCreature force-pushed the feature/gnu-message-style branch 5 times, most recently from 5873c6c to 544741c Compare January 17, 2020 11:29

FeepingCreature requested a review from ibuclaw as a code owner January 17, 2020 11:29

FeepingCreature force-pushed the feature/gnu-message-style branch 4 times, most recently from 096e704 to 7aa7cbd Compare January 17, 2020 11:41

FeepingCreature force-pushed the feature/gnu-message-style branch 2 times, most recently from 0fa96d3 to 2af15c8 Compare January 17, 2020 11:43

wilzbach reviewed Jan 17, 2020

View reviewed changes

FeepingCreature force-pushed the feature/gnu-message-style branch 2 times, most recently from 515361d to 013007a Compare January 17, 2020 12:22

Geod24 reviewed Jan 31, 2020

View reviewed changes

Implement issue 20515: add command line flag to allow selecting GNU m…

09a1a38

…essage style for errors and warnings.

FeepingCreature force-pushed the feature/gnu-message-style branch from 013007a to 09a1a38 Compare January 31, 2020 10:29

Geod24 added the Merge:auto-merge label Jan 31, 2020

thewilsonator added Merge:72h no objection -> merge The PR will be merged if there are no objections raised. and removed Merge:auto-merge labels Jan 31, 2020

WalterBright approved these changes Feb 1, 2020

View reviewed changes

RazvanN7 approved these changes Feb 3, 2020

View reviewed changes

RazvanN7 added Merge:auto-merge and removed Merge:72h no objection -> merge The PR will be merged if there are no objections raised. labels Feb 3, 2020

dlang-bot merged commit 79ad934 into dlang:master Feb 3, 2020

FeepingCreature deleted the feature/gnu-message-style branch February 3, 2020 07:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement issue 20515: add command line flag to allow selecting GNU message style for errors and warnings. #10732

Implement issue 20515: add command line flag to allow selecting GNU message style for errors and warnings. #10732

FeepingCreature commented Jan 17, 2020

dlang-bot commented Jan 17, 2020

FeepingCreature commented Jan 17, 2020

wilzbach left a comment

wilzbach Jan 17, 2020

FeepingCreature Jan 17, 2020 •

edited

Loading

wilzbach Jan 17, 2020

FeepingCreature commented Jan 17, 2020

wilzbach commented Jan 17, 2020 •

edited

Loading

FeepingCreature commented Jan 17, 2020 •

edited

Loading

FeepingCreature commented Jan 31, 2020

Geod24 left a comment

Geod24 Jan 31, 2020

FeepingCreature Jan 31, 2020

FeepingCreature commented Jan 31, 2020

FeepingCreature commented Jan 31, 2020

thewilsonator commented Jan 31, 2020

wilzbach commented Jan 31, 2020

FeepingCreature commented Jan 31, 2020 •

edited

Loading

wilzbach commented Jan 31, 2020

MoonlightSentinel commented Jan 31, 2020 •

edited

Loading

	Option("verror-style=digitalmars",
	Option("verror-style=[digitalmars\|gnu\|?]",

Implement issue 20515: add command line flag to allow selecting GNU message style for errors and warnings. #10732

Implement issue 20515: add command line flag to allow selecting GNU message style for errors and warnings. #10732

Conversation

FeepingCreature commented Jan 17, 2020

dlang-bot commented Jan 17, 2020

Bugzilla references

Testing this PR locally

FeepingCreature commented Jan 17, 2020

wilzbach left a comment

Choose a reason for hiding this comment

wilzbach Jan 17, 2020

Choose a reason for hiding this comment

FeepingCreature Jan 17, 2020 • edited Loading

Choose a reason for hiding this comment

wilzbach Jan 17, 2020

Choose a reason for hiding this comment

FeepingCreature commented Jan 17, 2020

wilzbach commented Jan 17, 2020 • edited Loading

FeepingCreature commented Jan 17, 2020 • edited Loading

FeepingCreature commented Jan 31, 2020

Geod24 left a comment

Choose a reason for hiding this comment

Geod24 Jan 31, 2020

Choose a reason for hiding this comment

FeepingCreature Jan 31, 2020

Choose a reason for hiding this comment

FeepingCreature commented Jan 31, 2020

FeepingCreature commented Jan 31, 2020

thewilsonator commented Jan 31, 2020

wilzbach commented Jan 31, 2020

FeepingCreature commented Jan 31, 2020 • edited Loading

wilzbach commented Jan 31, 2020

MoonlightSentinel commented Jan 31, 2020 • edited Loading

FeepingCreature Jan 17, 2020 •

edited

Loading

wilzbach commented Jan 17, 2020 •

edited

Loading

FeepingCreature commented Jan 17, 2020 •

edited

Loading

FeepingCreature commented Jan 31, 2020 •

edited

Loading

MoonlightSentinel commented Jan 31, 2020 •

edited

Loading