search

radkovo / jStyleParser

jStyleParser is a CSS parser written in Java. It has its own application interface that is designed to allow an efficient CSS processing in Java and mapping the values to the Java data types. It parses CSS 2.1 style sheets into structures that can be efficiently assigned to DOM elements. It is intended be the primary CSS parser for the CSSBox library. While handling errors, it is user agent conforming according to the CSS specification.
http://cssbox.sourceforge.net/jstyleparser/
GNU Lesser General Public License v3.0
92 stars 49 forks source link

Extensibitily 1.20 #20

Closed radkovo closed 9 years ago

hrj commented 9 years ago

@radkovo Hi! Just out of curiosity, what is this branch about?

radkovo commented 9 years ago

Hi, this is an effort by @bertfrees that should allow custom grammar extensions (e.g. custom css properties). There are some build issues remaining - it seems that it is not possible to create a fully modular ANTLR grammar without some source preprocessing (e.g. concatenating multiple ANTLR sources before compiling them). Unfortunatelly, I had to focus on other projects last few months so I didn't have time to finish this. I am sorry. I will take a new look on this asap.

bertfrees commented 9 years ago

FYI I'm already using these patches alongside a bunch of others in my own "1.20-p1" version (see https://github.com/snaekobbi/jStyleParser/pull/3). In that sense integrating this into the main branch isn't critical, but of course maintainability-wise it would still be good to have it merged ASAP.

radkovo commented 9 years ago

I have merged the refactoring made by @bertfrees and current master into a new branch merged_extensibility. It will become a new master unless you have some comments. I just didn't want to merge it to master directly for the case @hrj has some work in progress. All tests seem to be passing for me and I like the result of the refactoring very much - it makes the grammar files much clearer.

hrj commented 9 years ago

@radkovo Yes, I will make a PR for #30 soon. Will also do some testing with the merged_extensibility branch in gngr.

Aside, I suggest you do a rebase of the "extensibility" changes onto current master. This will reduce the number of criss-crossing commits, and it will be easier to see what changed. I am curious to see how the grammar changed. Of course, I can do it locally as well, in case you don't want to do it.

hrj commented 9 years ago

I have tested the merged_extensibility branch with gngr and all seems fine.

Also, a rebased version of that branch can be seen here.

Edit: The rebase was done on radkovo/jstyleparser/master branch.

radkovo commented 9 years ago

Many thanks for testing the merged branch.

Regarding rebase: I am not a git expert and have no practical experience with rebase but I remember some basic rules as "do not rebase the revisions already published to a public repository". It seems that this generally changes the history someone could rely on. Do you think it's safe and worth the possible problems? I am sorry for possible delayed answers since I'll be traveling the next week.

hrj commented 9 years ago

Re rebase:

You are right, that advice needs to be followed. But it is okay to rebase and publish to a new branch.

In a nut shell, rebase picks up some commits (based on some criteria), and then "replays" them in a new order. This creates new commits which look like the old ones, but from git's perspective they are new (because their ancestor commits are different or the line numbers are different). If you publish those new commits to the same branch then it will confuse everybody, but if you publish them under a new branch, then that is okay. The new branch is just like any other branch with fresh commits.

If you are not comfortable with rebase yet; it is fine. We can avoid it for now.

Just a reminder; small but useful PR #32 needs merging.

Cheers!