PHP-CSS-Parser
PHP-CSS-Parser copied to clipboard
[TASK] Use delegation for `DeclarationBlock` -> `RuleSet`
... rather than inheritance.
This will allow DeclarationBlock to instead extend CSSBlockList in order to support
CSS nesting.
This is a slightly-breaking change, since now CSSBlockList::getAllRuleSets() will include the RuleSet property of the DeclarationBlock instead of the DeclarationBlock itself.
Part of #1170.
coverage: 59.595% (-0.2%) from 59.816% when pulling e666f5e2f46f71b103b71db1f07fc3c343018198 on task/declarationblock-delegate-ruleset into b2028eb9519718a97c006c0536e0998f570b3bc7 on main.
Would be good to resolve #1247 before moving on with this.
This is almost ready for review, but some tests need to be added. Before doing so, I'd appreciate a pre-review from you: @sabberworm, @oliverklee.
I'm not keen on the having the various chaining methods in DeclarationBlock, but to avoid breaking changes they are necessary. A trait can be used for testing them in the same way as their counterparts in RuleSet.
For DeclarationBlock to contain nested rules, it needs to extend CSSBlockList. RuleSet cannot be a trait; it needs to be a fully-fledged type for methods like getAllRuleSets().
This is the only realistic way forward I see for now, in an 'Agile' sense, which can deliver what is required now, without wholescale refactoring or breaking changes.
Maybe as another spin-off PR (or prepatch), we can introduce the interface and have one class implement it.
Maybe as another spin-off PR (or prepatch), we can introduce the interface and have one class implement it.
#1256.
I think all pre-PRs that can be separated have been completed now.
This PR needs a rebase.
Apart from that, I think we can move de-abstracting the RuleSet class (and creating a dedicated testcase for it by copying some existing tests) into a pre-PR.
This PR needs a rebase.
It is impossible to rebase now, because git does it one commit at a time, and I probably included changes in the wrong commits in a previous rebase. I tried merge. Might be able to fix the resulting errors. Otherwise will have to start over.
I tried undoing lastest merge, but it's still a mess. I can't even get back to where I was now. Will take some time redoing.
I tried undoing lastest merge, but it's still a mess. I can't even get back to where I was now. Will take some time redoing.
In my experience, when I had a PR from which I spun off some pre-PRs, I also sometimes had nasty merge conflicts when rebasing. In those cases, it made things a lot easier if I squashed the existing commits in the PR branch before rebasing.
I tried undoing lastest merge, but it's still a mess. I can't even get back to where I was now. Will take some time redoing.
In my experience, when I had a PR from which I spun off some pre-PRs, I also sometimes had nasty merge conflicts when rebasing. In those cases, it made things a lot easier if I squashed the existing commits in the PR branch before rebasing.
I think I have somehow managed to rebase without including any changes from main. So now the diffs are showing that all the changes from main are being reverted. I did git rebase --abort when I got in a muddle.
If I could somehow reset the branch to the state it was in immediately after the commit "Reorder construction", then squashing may be possible, but I'm not sure how to do that.
If I could somehow reset the branch to the state it was in immediately after the commit "Reorder construction", then squashing may be possible, but I'm not sure how to do that.
git reflog allows you to list all recent commits and which actions lead to them, including rebases, branch switches etc. So this might be helpful. (It has saved my work a couple of times.)
If I could somehow reset the branch to the state it was in immediately after the commit "Reorder construction"
git reset -hard {SHA-of-last-commit-before-attempted-rebase}
... undid the failed rebase/merge. I think when I tried merging instead (via GitHub), that moved the 'common ancestor' point, but when I reverted the merge, it was not moved back.
then squashing may be possible, but I'm not sure how to do that.
git reset -soft {SHA-of-first-branch-commit}
git commit --amend
... did the squashing.
In my experience, when I had a PR from which I spun off some pre-PRs, I also sometimes had nasty merge conflicts when rebasing. In those cases, it made things a lot easier if I squashed the existing commits in the PR branch before rebasing.
That helped immensely; thanks. Instead of (presumably) 9 rounds of conflict resolution, including for files with no overall conflict, there was just one round, for the two files that actually had conflicts. (I'm used to Perforce, which does the merge all at once, and also has a visual editor for 3-way merging. Keeping a cool head also helps ;))
Rebase now done !) Thanks <3
I'm used to Perforce, which does the merge all at once, and also has a visual editor for 3-way merging.
I use PhpStorm as my IDE, and IIRC it also have a nice 3-way-merge UI. (Disclaimer: I don't use UIs for git stuff and hence can only report what I hear and see from other people.)
I think this is now ready for a final review.
This issues from the last review should all now be resolved.