-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
Add completion provider for CSS functions #1951
Add completion provider for CSS functions #1951
Conversation
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.13%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really cool to have function completion as well. Thanks a lot, @vanillajonathan,
I think the difference with the current css property completion provider are very small. I think we can DRY this code and have all completion code in the same module.
"attr", | ||
"calc", | ||
"conic-gradient", | ||
"counter", | ||
"cubic-bezier", | ||
"hsl", | ||
"hsla", | ||
"linear-gradient", | ||
"max", | ||
"min", | ||
"radial-gradient", | ||
"repeating-conic-gradient", | ||
"repeating-linear-gradient", | ||
"repeating-radial-gradient", | ||
"rgb", | ||
"rgba", | ||
"var", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should limit this to the functions that Gaphor supports: hsl
, hsla
, rgb
, and rgba
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I would guess anything that WebKit support would work?
I think the other functions would probably be possible to use too, even though I think they're unlikely to be much used, as many are for advanced uses and seldom used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I would guess anything that WebKit support would work?
We're not using webkit for rendering. Instead we use Cairo and a the items render themselves on the canvas, with some styling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I am aware which CSS properties and functions can be used in Cairo.
@@ -0,0 +1,100 @@ | |||
from gi.repository import Gio, GObject, Gtk, GtkSource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Gaphor we tend to use file names of concatenated words, no undescore (e.g. treecomponent.py
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So then this would be cssfunctioncompletionprovider.py
, yuck!
Quite hard to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both CompletionProvider classes can be added to csscompletion.py
.
We have quite some cases where more than one class is added to a module. It allows us to group similar concepts cleanly. We're not writing Java, after all 😃 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True!
from gi.repository import Gio, GObject, Gtk, GtkSource | ||
|
||
|
||
class CssFunctionCompletionProvider(GObject.GObject, GtkSource.CompletionProvider): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code for this class is almost a copy of CssPropertyCompletionProvider
. I think it's pretty simple to create a simple parameterizable version of a completion provider. Then both completion provider can be in the same file (gaphor/ui/csscompletion.py
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is, right now, but but GtkSourceView is built to support multiple active completion providers, and we can modify these so they differ from each other.
Example it could append ()
after the function name when inserted. It could append a :
after the attribute name when inserted.
They also have different do_get_title
and do_get_priority
. The title isn't used by GtkSourceView today, but in future versions it might be. The priority can be used for ordering, example so we can have attributes ordered before functions.
Later, we can add completion providers for colors (they will have a different priority too), and for units for which we might to present when a number is entered, so it is good to keep completions for different things in different completion providers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of appending ()
and :
.
The priority can be used for ordering, example so we can have attributes ordered before functions.
Thinks like title and priority can be past as constructor parameters to a generic base class, where we handle the common boilerplate.
Later, we can add completion providers for colors (they will have a different priority too), and for units for which we might to present when a number is entered, so it is good to keep completions for different things in different completion providers.
I see your point. Indeed completers could defer. However, I would not focus to much on later 😉 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of appending () and :.
The appending of characters upon insertion would be done in the do_activate
method.
Different providers could have different implementations for the is_trigger
method, e.g. to show units (such as px
) when the user types a digit.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information