-
Notifications
You must be signed in to change notification settings - Fork 177
Implements a StyleProcessor class to allow access to a list of set styles for a widget in String format. #2392
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
base: master
Are you sure you want to change the base?
Conversation
Styleprocessor branch
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.
Given the implementation details I see, could this not all be done externally without introducing APIs in SWT?
* @return ArrayList<String> styles | ||
* @since 3.131 | ||
*/ | ||
public ArrayList<String> getStyles() { |
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.
One would never want to surface the List implementation class in any API; elsewhere too.
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.
Hello @merks,
Answering to your question...
Given the implementation details I see, could this not all be done externally without introducing APIs in SWT?
We initially explored other approaches such as implementing it in a utility class or using annotations, however based on the comments and guidance that we got Issue #611. We were advised to directly change the SWT repository, specifically the widget classes instead. That being said, we are flexible and happy to revise the implementation.
One would never want to surface the List implementation class in any API; elsewhere too.
You are right with what you've said. My teammate and I are planning to change this ArrayList to List instead and avoiding making the method public. We've only did it this way for testing purposes to see if it works and get feedback.
Thank you so much!
Teammate - @WillPeltz
|
||
import org.eclipse.swt.*; | ||
|
||
public class StyleProcessor { |
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.
Will this really become public API or can it just be package protected?
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.
Hello @merks,
You're absolutely right, we actually intended for StyleProcessor to be package private, so that it can only be accessed within the widgets package. My teammate and I forgot that part, thank you for clarifying.
Thank you so much!
Teammate - @WillPeltz
The recommendation was to access this reflectively and not to make any public API here. |
This PR allows us to fix issues with the CSS Spy in Eclipse PDE, detailed in Issue #611 Completed with @WillPeltz
Changes
-StyleProcessor class
Parses styles that can be set based on context, checks relevant styles against integer bitmask to see which are actually set, and converts them into String list
-getStyles method in Text and Button classes
Returns a list of styles currently set for a given widget in String format, allowing for style inspection at runtime
-Static function using instance of StyleProcessor in Text and Button classes
Clearly defines which styles can be set for a widget based on context, in a human-readable way.
Motivation
Currently, there is no built-in way to view a widget’s set styles in a human-readable format, leading to projects like Eclipse PDE implementing complex and limited parsers for UI elements like the CSS Spy tool. This PR makes this process much simpler and easier, without requiring much upkeep. It should be quite simple to add support for and update widget classes.
Notes
-Since this implementation only focuses on Text and Button classes, other widget classes should be revised incrementally to use the StyleProcessor class.
-Backwards compatible; existing behavior is unchanged.
Having both getStyles and checkStyle is a bit redundant, so merging them at some point in the future might be a good idea if it can be done without breaking existing behavior too much in downstream projects.