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 17396 - Add colorized syntax highlighting to error messages… #6777

Merged
merged 1 commit into from
May 14, 2017

Conversation

WalterBright
Copy link
Member

… emitted by dmd

This colorizes D code in between backticks in error messages.

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
17396 Add colorized syntax highlighting to error messages emitted by dmd

@WalterBright
Copy link
Member Author

Anyone have any idea why this is suddenly happening?

ddmd/backend/cc.d(15): Error: module dlist is in file 'tk/dlist.d' which cannot be read

The file is certainly there.

@andralex
Copy link
Member

cc @MartinNowak

@rainers
Copy link
Member

rainers commented May 13, 2017

Anyone have any idea why this is suddenly happening?

dlist.d is not on the command line of the library being compiled, so the module name should match the folder path, i.e. it should be module ddmd.tk.dlist, not tk.dlist.

@WalterBright
Copy link
Member Author

dlist.d is not on the command line of the library being compiled, so the module name should match the folder path, i.e. it should be module ddmd.tk.dlist, not tk.dlist

Ok, but why is this suddenly happening? Why is it happening to this PR and not the others?

@WalterBright WalterBright force-pushed the fix17396 branch 2 times, most recently from ffb8503 to 101255d Compare May 13, 2017 22:35
@rainers
Copy link
Member

rainers commented May 13, 2017

Ok, but why is this suddenly happening? Why is it happening to this PR and not the others?

You have added an import ddmd.doc to the lexer library, which now imports more or less the rest of the compiler.

@WalterBright
Copy link
Member Author

Oh, ok. That makes sense. Thanks!

@WalterBright
Copy link
Member Author

I'll leave it to someone with better color sense to pick better colors.

@WalterBright WalterBright force-pushed the fix17396 branch 3 times, most recently from d629708 to 13a0a4b Compare May 13, 2017 23:09
@WalterBright
Copy link
Member Author

It's working now! Of course, the rest of the error messages need to be marked with backticks for it to work with them.

@andralex andralex merged commit 34d0259 into dlang:master May 14, 2017
@CyberShadow
Copy link
Member

@WalterBright Something is wrong here, looks like newlines are getting lost somewhere:

@WalterBright
Copy link
Member Author

yeah, fixed that in #6781

@CyberShadow
Copy link
Member

yeah, fixed that in #6781

Confirmed, thanks.

@CyberShadow
Copy link
Member

I'll leave it to someone with better color sense to pick better colors.

For one I think we should avoid red and leave that reserved for the "Error:" part.

@ibuclaw
Copy link
Member

ibuclaw commented May 15, 2017

Even just having them bold would be enough IMO.

@ibuclaw
Copy link
Member

ibuclaw commented May 15, 2017

For one I think we should avoid red and leave that reserved for the "Error:" part.

You could make the same argument for Warning and Deprecation messages too. Also supplemental and speculative messages if they get different highlights. That doesn't really leave much else left to use.

@ibuclaw
Copy link
Member

ibuclaw commented May 15, 2017

I'll leave it to someone with better color sense to pick better colors.

I may be biased towards cgdb here, but if you insist on it being coloured, the following groupings I find are the least surprising:

  • keyword: blue (ie: __traits, mixin, foreach)
  • type: green (ie: Object, int, final)
  • literal: red (ie: "string", 123, null)
  • directive: cyan (ie: module, pragma, @safe)
  • comment: yellow

There may be some who may want to swap token and comment colours in my above list.

@grogancolin
Copy link

grogancolin commented May 15, 2017

I also think a dmd.conf parameter to turn off colors globally is desirable.

Colors mess with some folks terminals (different color schemes etc) - it should be easily disabled. (And preferrably not by adding --no-color or similar on every command)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants