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

Update css block regex to allow final semicolon omission #3032

Merged
merged 3 commits into from
Jun 28, 2022

Conversation

VivaLaVia
Copy link
Contributor

@VivaLaVia VivaLaVia commented Jun 1, 2022

Update the css regex to accept css blocks that do not have a final
semicolon on the last rule. In cases where a minified stylesheet is
used, its common that this semicolon is omitted.

Associated issues: #3031

Checklist

Author:

  • The proper base branch has been selected. New features go on unstable. Bug-fix patches can go on either unstable or master.
  • Automated tests have been included in this pull request, if possible, for the new feature(s) or bug fix. Check this box if tests are not pragmatically possible (e.g. rendering features could include screenshots or videos instead of automated tests).
  • The associated GitHub issues are included (above).
  • Notes have been included (above).

Reviewers:

  • All automated checks are passing (green check next to latest commit).
  • At least one reviewer has signed off on the pull request.
  • Just after this pull request is merged, it should be applied to both the master branch and the unstable branch. Normally, this just requires cherry-picking the corresponding merge commit from master to unstable -- or vice versa.

@maxkfranz
Copy link
Member

This looks like a nice feature. Since this is not rendering-related, there should be some unit tests for this before we merge this in. At the very least, there should be a basic sanity check that the feature is working as expected. Are there edge cases to consider?

@maxkfranz maxkfranz linked an issue Jun 7, 2022 that may be closed by this pull request
7 tasks
@maxkfranz
Copy link
Member

It looks like the documentation should also be updated to include your new behaviour. It currently says this:

String format

Note that the trailing semicolons for each property are mandatory. Parsing will certainly fail without them.

https://js.cytoscape.org/#style/format

@maxkfranz maxkfranz changed the base branch from master to unstable June 7, 2022 14:18
@maxkfranz maxkfranz added this to the 3.22.0 milestone Jun 7, 2022
@VivaLaVia VivaLaVia force-pushed the fix/style-block-regex branch from a8fbc17 to a06af0b Compare June 10, 2022 20:30
@VivaLaVia VivaLaVia marked this pull request as ready for review June 12, 2022 19:44
@VivaLaVia
Copy link
Contributor Author

Updated the documentation and added some test for happy/unhappy paths for string sheet semicolon parsing. Please let me know if I goofed any merge conflictions with the changes from master. If needed, I can recreate and fork from the correct branch

@maxkfranz
Copy link
Member

Updated the documentation and added some test for happy/unhappy paths for string sheet semicolon parsing.

Great

Please let me know if I goofed any merge conflictions with the changes from master. If needed, I can recreate and fork from the correct branch

That would be helpful. Maybe rebasing your branch to the tip of unstable would clean it up without having to start all over and cherry-pick everything

Update the css regex to accept css blocks that do not have a final
semicolon on the last rule. In cases where a minified stylesheet is
used, its common that this simicolon is omitted.
@VivaLaVia VivaLaVia force-pushed the fix/style-block-regex branch from 77ed6f1 to 0c22210 Compare June 22, 2022 02:02
@VivaLaVia
Copy link
Contributor Author

Should be all set now. Let me know if you need any further changes

@maxkfranz maxkfranz merged commit 89ba597 into cytoscape:unstable Jun 28, 2022
@maxkfranz
Copy link
Member

Nice.

This is now in the unstable branch, which will be released with 3.22.0 on 5 July 2022.

@VivaLaVia VivaLaVia deleted the fix/style-block-regex branch June 29, 2022 13:07
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

Successfully merging this pull request may close these issues.

String sheet styles ignored when minified
2 participants