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.

Sign up for GitHub

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

Jump to bottom

Enhance Highlighting Customization for OpenFgaDsl Language #244

Closed
wants to merge 1 commit into from
Closed

Enhance Highlighting Customization for OpenFgaDsl Language #244

wants to merge 1 commit into from

Conversation

Siddhant-K-code
Copy link

@Siddhant-K-code Siddhant-K-code commented Apr 14, 2024 •
edited
Loading

Description

This PR aims to enhance the customization of syntax highlighting for the OpenFgaDsl language definition.

Changes:

  • Restructured the tokens in the language definition to use the default PrismJS tokens.
  • Added alias mappings to maintain the connection with the original OpenFgaDslThemeTokenType enum values.

This change will make it easier for users to customize the highlighting colors without needing to add the OpenFGA specific CSS. It provides a more seamless integration with the default PrismJS themes and improves the overall DevX when working with OpenFgaDsl code.

Why this approach?

In this, each token in the languageDefinition object has been given a more descriptive name that aligns with the default PrismJS tokens. The alias property is used to maintain the mapping to the original OpenFgaDslThemeTokenType enum values.

This approach allows for easier customization of the highlighting colors by utilizing the default PrismJS tokens, while still keeping the internal mapping to the specific OpenFgaDslThemeTokenType enum values.

References

fixes #177

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@Siddhant-K-code
feat(tools/prism): Restructure tokens for easier highlighting.
53c9df0 
Restructure the tokens in the language definition to use the default PrismJS tokens for easier customization of highlighting colors.
@Siddhant-K-code Siddhant-K-code requested review from a team as code owners April 14, 2024 05:52
rhamzeh
rhamzeh reviewed Apr 18, 2024
View reviewed changes
src/tools/prism/language-definition.ts
@@ -1,26 +1,29 @@
import { OpenFgaDslThemeTokenType } from "../../theme/theme.typings";

export const languageDefinition = {
[OpenFgaDslThemeTokenType.MODULE]: {
module: {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @Siddhant-K-code!

Can you elaborate on the change from using the enums to strings? Not sure how that helps

Copy link
Author

Choose a reason for hiding this comment

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

Why this approach?
In this, each token in the languageDefinition object has been given a more descriptive name that aligns with the default PrismJS tokens. The alias property is used to maintain the mapping to the original OpenFgaDslThemeTokenType enum values.

This approach allows for easier customization of the highlighting colors by utilizing the default PrismJS tokens, while still keeping the internal mapping to the specific OpenFgaDslThemeTokenType enum values.

I've tried to explain this approach in PR description. Let me know, if anything is clear or I could explain in better 🙏🏼

@Siddhant-K-code Siddhant-K-code deleted the fix/177 branch April 22, 2024 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhamzeh rhamzeh rhamzeh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restructure the Prismjs tokens to re-use the Prismjs default tokens when possible
2 participants
@Siddhant-K-code @rhamzeh