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

Multiline labels which start or end on the same line should have their lines merged #265

Open
RDambrosio016 opened this issue Jul 14, 2020 · 5 comments

Comments

@RDambrosio016
Copy link
Contributor

Currently if you have two multiline labels, which both start or end on the same line, the rendering gets kind of wonky

error[no-empty]: Empty block statements are not allowed
  ┌─ tests\main.js:1:5
  │
1 │     try {
  │ ┌───────^
2 │ │   
3 │ │   } catch (e {
  │ └───^
  │   ┌───'
4 │   │ 
5 │   │ } finally {
  │   └─' This handler is unreachable

Instead, the lines should be merged into one line as follows, with the lines after the first label being the color of the second label, in this case cyan, this would illustrate more clearly that the label does in fact start there

error[no-empty]: Empty block statements are not allowed
  ┌─ tests\main.js:1:5
  │
1 │     try {
  │ ┌───────^
2 │ │   
3 │ │   } catch (e {
  │ └─┬─^─'
4 │   │ 
5 │   │ } finally {
  │   └─' This handler is unreachable
@brendanzab
Copy link
Owner

brendanzab commented Jul 15, 2020

Would this be ok?

error[no-empty]: Empty block statements are not allowed
  ┌─ tests\main.js:1:5
  │
1 │     try {
  │ ╭───────^
2 │ │   
3 │ │   } catch (e {
  │ ╰───^─'
4 │   │ 
5 │   │ } finally {
  │   ╰─' This handler is unreachable

Ie. due to the limitations of terminal rendering we can't render the border intersection with multiple colours.

@RDambrosio016
Copy link
Contributor Author

Yeah i realize that you cant have them be the same color, however, i was thinking of two possible solutions:

  • If the colors for primary and secondary are default, and this isnt a windows console you could blend them into purple using termcolor Rgb(..) which i think would look nice
  • Or else just make the color of the junction the color of the first line, so red because of primary

I think the way you proposed looks sort of disconnected, but thats just my opinion 🙂

@brendanzab
Copy link
Owner

Yeah I'm really not a fan of either of those options TBH. In the first case you'd have a single character that is a different colour from the rest. In the second you'd have a distracting 'spur' of the primary label colour bleeding into the secondary underline. It does look disconnected, but it will look a bit better when there is some colour linking the two bits of the secondary line.

@RDambrosio016
Copy link
Contributor Author

Yeah i understand, on second thought your proposal sounds better. its disconnected, however, thinking about it, a weird coloring for the bottom bit looks more frustrating and ugly 👍

@Johann150
Copy link
Collaborator

I think this should stay how it is. The proposal would make the rendering depend on colour output. If there is uncoloured output (or accessibility: colourblind) it looks really confusing, as can be seen in the examples given here.

Still, in the given examples one could differentiate between the labels because the first is a primary and the other is a secondary label and they thus use different characters for rendering (primary uses ^ and secondary uses '). But if both were primary or secondary labels, it could easily look like the following, which is just more confusing:

error[no-empty]: Empty block statements are not allowed
  ┌─ tests\main.js:1:5
  │
1 │     try {
  │ ╭───────^
2 │ │
3 │ │   } catch (e {
  │ ╰───^─^
4 │   │
5 │   │ } finally {
  │   ╰─^ This handler is unreachable

Although I have to admit that ^─^ looks nice, as a whole this looks more like a downgrade to me.

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

No branches or pull requests

3 participants