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

Use style instead of colours for the completion list #617

Closed
gnodet opened this issue Dec 9, 2020 · 6 comments
Closed

Use style instead of colours for the completion list #617

gnodet opened this issue Dec 9, 2020 · 6 comments
Assignees

Comments

@gnodet
Copy link
Member

gnodet commented Dec 9, 2020

No description provided.

@gnodet
Copy link
Member Author

gnodet commented Dec 9, 2020

The style strings should be final and not changed after being converted to a style.
So instead of

        if (menuList) {
            return AttributedStyle.DEFAULT.background(getCompletionListBackgroundColor());
        }
        return AttributedStyle.DEFAULT.backgroundDefault();

I think we should duplicate the styles names and have COMPLETION_STYLE_LIST_GROUP in addition of COMPLETION_STYLE_GROUP for example.

@mattirn Thoughts ?

@gnodet gnodet self-assigned this Dec 9, 2020
@mattirn
Copy link
Collaborator

mattirn commented Dec 9, 2020

Sure, the choice of colour as integer is bad especially if you are going to add support for true colour for the rest I think I have just modified the existing implementation.

I do not know what we can exactly set using style strings and how...
For example if we can define both foreground and background in a style then we can duplicate all the styles that we use inside menu list.
On the other hand if style can define only foreground or background colour then I think we need to duplicate only menu list background colour.

@gnodet
Copy link
Member Author

gnodet commented Dec 9, 2020

Fwiw, the styles are the ansi representation of whatever the terminal support (foreground, background, bold, blink, crossed out, italic, underling...). So you can do whatever you want, the problem is that it's not really intuitive.
I'll first add some javadoc, but maybe we could find something more human readable than a list of integers... Ideally, we should leverage the StyleResolver.

@mattirn
Copy link
Collaborator

mattirn commented Dec 9, 2020

Not tested much but seems to work... mattirn@63b5c11

@gnodet
Copy link
Member Author

gnodet commented Dec 9, 2020

I have something that leverages the StyleResolver, I'll try to push it tomorrow, but it's currently merged with the color support, so I need to split those...

@mattirn
Copy link
Collaborator

mattirn commented Dec 10, 2020

About human friendly styles also the latest nanorc format is quite good

[style1,style2,...,stylen,][foreground][,background]

where stylei are bold, inverse, italic, underline,... . foreground and background are colours. Everything in style string is optional. Nano.java#L1822-L1837

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