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

fix issue 20824 - error messages generated by dmd build script can be… #11209

Merged
merged 1 commit into from Jun 2, 2020
Merged

fix issue 20824 - error messages generated by dmd build script can be… #11209

merged 1 commit into from Jun 2, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 2, 2020

… prefixed with a non-standard "ERROR:"

So that dmd developpers working with IDE can click the first message.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @NilsLankila! 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
20824 normal error messages generated by dmd build script can be prefixed with a non-standard "ERROR:"

Testing this PR locally

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

dub run digger -- build "master + dmd#11209"

@ghost
Copy link
Author

ghost commented Jun 2, 2020

How about just dropping this Error: the string "error" will be seen elsewhere, or when not a stack trace... makingo obvious that there has been an error (in addition to the script return code...)

@MoonlightSentinel
Copy link
Contributor

That sounds even better than the current newline approach.

@ghost
Copy link
Author

ghost commented Jun 2, 2020

we have a test suite regression (unrelated to this change):

Test fail_compilation/fail7524a.d failed: 
expected:
----
fail_compilation/fail7524a.d(10): Error: #line integer ["filespec"]\n expected
fail_compilation/fail7524a.d(10): Error: declaration expected, not `"$r:\w+ \d+ \d+$"`
----
actual:
----
fail_compilation/fail7524a.d(10): Error: #line integer ["filespec"]\n expected
fail_compilation/fail7524a.d(10): Error: declaration expected, not `"Jun  1 2020"`
----
diff:
----
 fail_compilation/fail7524a.d(10): Error: #line integer ["filespec"]\n expected
-fail_compilation/fail7524a.d(10): Error: declaration expected, not `"$r:\w+ \d+ \d+$"`
+fail_compilation/fail7524a.d(10): Error: declaration expected, not `"Jun  1 2020"`

@thewilsonator
Copy link
Contributor

Please rebase it should go away.

@MoonlightSentinel
Copy link
Contributor

Fixed by #11206 which was merged a few minutes ago. The error will vanish on rebase.

… prefixed with a non-standard "ERROR:"

So that dmd developpers working with IDE can click the first message.
@ghost
Copy link
Author

ghost commented Jun 2, 2020

Yes, in first place i've edited online so the parent commit of my fix was more than a week old (when forking after registration) . Should be good now.

@dlang-bot dlang-bot merged commit af49d06 into dlang:master Jun 2, 2020
@ghost ghost deleted the patch-1 branch June 2, 2020 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants