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

Inheritance from :default gives surprising merge behavior #97

Open
topolarity opened this issue Oct 14, 2024 · 5 comments
Open

Inheritance from :default gives surprising merge behavior #97

topolarity opened this issue Oct 14, 2024 · 5 comments

Comments

@topolarity
Copy link
Member

This merge surprisingly destroys the underline status:

julia> merge(StyledStrings.getface(:underline), StyledStrings.getface(:red))
StyledStrings.Face (sample)
          font: monospace
        height: 120
        weight: normal
         slant: normal
    foreground: ■ red
    background: ■ default
     underline: false
 strikethrough: false
       inverse: false

Printing also behaves this way, but only if you make the inheritance explicit:

My take-away from this is that Faces don't / shouldn't inherit from :default. Instead printing uses :default as the implicit styling start to start with (which is similar but not the same)

@tecosaur
Copy link
Collaborator

tecosaur commented Oct 15, 2024

Two quick things:

  1. I can't see the image, that might have something to do with the URL starting with https://private-user-images.githubusercontent.com
  2. Yep, this is because getface fully resolves all face attributes, but maybe this behaviour should be a keyword option? The idiomatic way of doing this internally is merge(FACES.current[][:underline], FACES.current[][:red]) which produces the expected:
    Face (sample)
        foreground: ■ red
         underline: true
    
    This is also the result of getface([:underline, :red]).

@topolarity
Copy link
Member Author

Let me try the image again:
Image

@tecosaur
Copy link
Collaborator

tecosaur commented Oct 15, 2024

Hmmm, at a glance that behaviour looks reasonable to me. Regarding the difference in the pattern here bit, in the first example you say you want to apply a face that consists of default with the foreground overriden to yellow, in the second you say you want it to be yellow. These are different things, similarly:

Image

To me this seems like predictable, consistent behaviour. If I'm missing something do elaborate 🙂.

I suspect you may have intended the example not to show "surprising behaviour" but to show a perceived difference in behaviour when printing compared to merge + getface? In that case, I see what you mean and see how one could expect merge(getface(:a), getface(:b), getface(:c)) to be equivalent to getface([:a, :b, :c]) ... but this is not the case. I'm not sure if/what would be worth changing in particular.

@topolarity
Copy link
Member Author

topolarity commented Oct 15, 2024

The surprising part is that the docs say that all Faces inherit from :default, but when I do that explicitly it behaves differently

My take-away from this is that Faces don't / shouldn't inherit from :default. Instead printing uses :default as the implicit styling start to start with (which is similar but not the same)

@tecosaur
Copy link
Collaborator

Ah, I think I follow better now. I suppose a more accurate way to put it than the current docs is that all faces are "grounded" by the :default attributes, which is done through the same mechanism as inheritance, but applied a little differently.

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

2 participants