Skip to content

Add backquotes to have better looking rust code#24523

Merged
bors merged 2 commits into
rust-lang:masterfrom
GuillaumeGomez:clean-error-codes
Apr 24, 2015
Merged

Add backquotes to have better looking rust code#24523
bors merged 2 commits into
rust-lang:masterfrom
GuillaumeGomez:clean-error-codes

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

No description provided.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@ftxqxd

ftxqxd commented Apr 17, 2015

Copy link
Copy Markdown
Contributor

I don’t think these messages are Markdown (they’re just printed to the terminal), so why would backticks help here? Perhaps it would be better to simply indent the code, e.g.:

Foo bar baz:

    let foo = 1;
    let bar = foo;

Fizzbar foobang qux, frob quxbaz.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Then why did I have to add some (here) ? Kinda weird.

@ftxqxd

ftxqxd commented Apr 17, 2015

Copy link
Copy Markdown
Contributor

I disagree with @Manishearth there. I don’t think any non-Markdown code should be using to represent code blocks. Indentation is OK, and actually looks clearer in my opinion than using. That said, I’d still like to hear @Manishearth’s reasoning about backticks being easier to read.

@Manishearth

Copy link
Copy Markdown
Member

@P1start My point is that there should be something distinguishing code, especially block code, from regular text. It helps the flow of reading.

Perhaps @steveklabnik has some opinions on this ?

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Maybe we could use "```" as separator to print console color codes ? I'd be interested in implementing such a thing.

@Manishearth

Copy link
Copy Markdown
Member

Hm, so the code printed should be slightly greyer or something?

We'd need to pick out the existing foreground color and darken/lighten it a bit.

Alternatively, we can indent it. That's what man pages do (and they bold code)

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Yes, that's what I was thinking about.

@huonw

huonw commented Apr 17, 2015

Copy link
Copy Markdown
Contributor

It makes a lot of sense to me for these errors to also have a prettier web/rendered interface, which would work best if we formatted the messages as real markdown.

Of course block code can be denoted by indentation and that probably works best here.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

@huonw: I think it would be better to use a separator (like the current or another, doesn't matter) and then make a pretty printing in console (since that's where it will be displayed).

@Manishearth

Copy link
Copy Markdown
Member

So a concrete solution:

When printing to any terminal: Indent all code blocks in the diagnostics

When printing to a color-supported terminal (i.e. not to file), additionally mark inline code blocks as bold.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

I agree with @Manishearth's proposition. However, how do you intent to let know what part of the solution will have a "special" print ? For the moment, it's "```" and I'm fine with it but someone else maybe would want another one ?

@Manishearth

Copy link
Copy Markdown
Member

As in?

@huonw

huonw commented Apr 17, 2015

Copy link
Copy Markdown
Contributor

My point was that they won't always be displayed in the terminal. Also, if we want to do fancy formatting in the terminal, there's no particular reason we can't do that by parsing and "rendering" real markdown (I.e. with a proper parser etc) rather than an even more adhoc markup language.

We could even do syntax highlighting of code like rustdoc.

@Manishearth

Copy link
Copy Markdown
Member

I guess we should do indentation for now, and leave the rest to a separate issue?

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Well, I can change my current PR depending on what you need (I'm just speaking about the indentation or the "```"). However, with an indentation, the 80 columns "limit" will be reached faster.

@michaelsproul

Copy link
Copy Markdown
Contributor

I like Huon's idea of using Markdown and then outputting it appropriately for the different backends. I think we should leave everything the same for now and update to Markdown syntax once the infrastructure is in place. I've started working on something similar already, so I'll add this as a goal.

@bors

bors commented Apr 18, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #24562) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

So ?

@Manishearth

Copy link
Copy Markdown
Member

So...change the diagnostics printer to indent code blocks :)

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Done ! :-)

@Manishearth

Copy link
Copy Markdown
Member

I meant that we should modify the code responsible for displaying these strings to indent code fences, that's a more resilient solution.

@huonw what do you think?

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Oh right ! Well, I can do that if no one's working on it. Should I revert my last commit ?

@huonw

huonw commented Apr 21, 2015

Copy link
Copy Markdown
Contributor

That sounds good to me, although landing the back-ticks themselves first/separately is fine by me.

Maybe @steveklabnik has opinions.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

I added back the backquotes. I'll remove useless commits when I'll be back home.

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2ddc8f5 with merge 85e57ee...

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 24, 2015
@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⛄ The build was interrupted to prioritize another pull request.

bors added a commit that referenced this pull request Apr 24, 2015
@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2ddc8f5 with merge 0c17593...

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⛄ The build was interrupted to prioritize another pull request.

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2ddc8f5 with merge 812b3df...

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⛄ The build was interrupted to prioritize another pull request.

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2ddc8f5 with merge 910e942...

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⛄ The build was interrupted to prioritize another pull request.

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2ddc8f5 with merge a4498d4...

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 24, 2015
@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

⛄ The build was interrupted to prioritize another pull request.

bors added a commit that referenced this pull request Apr 24, 2015
@bors bors merged commit 2ddc8f5 into rust-lang:master Apr 24, 2015
@GuillaumeGomez GuillaumeGomez deleted the clean-error-codes branch April 26, 2015 12:44
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 16, 2015
…hton

 A merge in rust-lang#24523  broke the explanation for E0303. This commit restores the previous version and also removes an erroneous `&` (which had nothing to do with the merge).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants