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

Added color to bold selection style. #2324

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

soerendomroes
Copy link
Collaborator

This solution should be improved once the discussion in RFC 3 lf-lang/rfcs#3 completes.

@soerendomroes soerendomroes added the diagrams Problems with diagram synthesis label Jun 17, 2024
@lhstrh lhstrh added the feature New feature label Jun 17, 2024
Copy link
Collaborator

@cmnrd cmnrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Considering that the highlighting isn't perfect (e.g. for ports) and we still have some discussion to do, would it be possible to make this configurable? E.g. by adding an option "Colored highlightling"?

@soerendomroes
Copy link
Collaborator Author

Thanks! Considering that the highlighting isn't perfect (e.g. for ports) and we still have some discussion to do, would it be possible to make this configurable? E.g. by adding an option "Colored highlightling"?

Could you add this idea to the rfc?

@cmnrd
Copy link
Collaborator

cmnrd commented Jul 2, 2024

I see this more as a step towards getting this PR merged without breaking anything, before we agree on a roadmap in the RFC. Think of it as a feature flag.

@@ -292,6 +292,7 @@ public List<SynthesisOption> getDisplayedSynthesisOptions() {
ModeDiagrams.INITIALLY_COLLAPSE_MODES,
SHOW_USER_LABELS,
SHOW_HYPERLINKS,
LinguaFrancaStyleExtensions.SELECTION_HIGHLIGHTING_COLOR,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ordered the selection highlighting option below the more visual synthesis options. Feel free to comment on this

@@ -76,6 +78,12 @@ public class LinguaFrancaStyleExtensions extends AbstractSynthesisExtensions {
@Inject @Extension private KPolylineExtensions _kPolylineExtensions;
@Extension private KRenderingFactory _kRenderingFactory = KRenderingFactory.eINSTANCE;

public static final String SELECTION_HIGHLIGHTING_COLOR_LABEL = "Selection Coloring";
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose this label for the synthesis option.

Comment on lines +83 to +86
public static final SynthesisOption SELECTION_HIGHLIGHTING_COLOR =
SynthesisOption.createCheckOption(SELECTION_HIGHLIGHTING_COLOR_LABEL, false)
.setCategory(LinguaFrancaSynthesis.APPEARANCE);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the new synthesis option is in the appearance category and its default value is false such that it will not break things but can be enabled.

Comment on lines +97 to +101
// Improve this with content from https://github.com/lf-lang/rfcs/pull/3
boolean selectionColor = getBooleanValue(SELECTION_HIGHLIGHTING_COLOR);
if (selectionColor) {
_kRenderingExtensions.setSelectionForeground(r, Colors.ORANGE_1);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply add the selection foreground rendering to renderings if the synthesis option is enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagrams Problems with diagram synthesis feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants