- Notifications
You must be signed in to change notification settings - Fork 148
[FEATURE] Add support for the dvh
, lvh
and svh
length units #415
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @KaiOnGitHub, thanks for this PR!
Would you be willing to cover it with a unit test?
@oliverklee I fixed the char limit but unfortunately don't have the time to write a test atm. I tested it within our Shopware 6 application and with this change dvh worked while it did not before :) |
@KaiOnGitHub, thanks for providing the code changes and testing it within your app. If you still don't have time to add the unit tests, I may be able to pick this up - please don't delete your branch! |
@oliverklee, I tried to rebase this branch, but couldn't figure out what I was supposed to do. This time, I will simply re-apply the changes as a separate PR. But for future reference, what was I doing wrong here?: d:\git_files\MyIntervals\PHP-CSS-Parser>git pull remote: Enumerating objects: 1, done. remote: Counting objects: 100% (1/1), done. remote: Total 1 (delta 0), reused 1 (delta 0), pack-reused 0 Unpacking objects: 100% (1/1), 521 bytes | 57.00 KiB/s, done. From https://github.com/MyIntervals/PHP-CSS-Parser * [new tag] v8.5.1 -> v8.5.1 Already up to date. d:\git_files\MyIntervals\PHP-CSS-Parser>git fetch origin pull/415/head:pr/415 remote: Enumerating objects: 10, done. remote: Counting objects: 100% (10/10), done. remote: Compressing objects: 100% (6/6), done. remote: Total 10 (delta 4), reused 6 (delta 4), pack-reused 0 Unpacking objects: 100% (10/10), 3.81 KiB | 69.00 KiB/s, done. From https://github.com/MyIntervals/PHP-CSS-Parser * [new ref] refs/pull/415/head -> pr/415 d:\git_files\MyIntervals\PHP-CSS-Parser>git checkout pr/415 Switched to branch 'pr/415' d:\git_files\MyIntervals\PHP-CSS-Parser>git checkout main Switched to branch 'main' Your branch is up to date with 'origin/main'. d:\git_files\MyIntervals\PHP-CSS-Parser>git pull Already up to date. d:\git_files\MyIntervals\PHP-CSS-Parser>git checkout pr/415 Switched to branch 'pr/415' d:\git_files\MyIntervals\PHP-CSS-Parser>git pull There is no tracking information for the current branch. Please specify which branch you want to rebase against. See git-pull(1) for details. git pull <remote> <branch> If you wish to set tracking information for this branch you can do so with: git branch --set-upstream-to=origin/<branch> pr/415 d:\git_files\MyIntervals\PHP-CSS-Parser>git branch --set-upstream-to=origin/main pr/415 branch 'pr/415' set up to track 'origin/main'. d:\git_files\MyIntervals\PHP-CSS-Parser>git pull Auto-merging src/Value/Size.php CONFLICT (content): Merge conflict in src/Value/Size.php error: could not apply 16b5f18... Add support for dvh, svh, lvh. Close #412 hint: Resolve all conflicts manually, mark them as resolved with hint: "git add/rm <conflicted_files>", then run "git rebase --continue". hint: You can instead skip this commit: run "git rebase --skip". hint: To abort and get back to the state before "git rebase", run "git rebase --abort". Could not apply 16b5f18... Add support for dvh, svh, lvh. Close #412 d:\git_files\MyIntervals\PHP-CSS-Parser>git checkout main error: you need to resolve your current index first src/Value/Size.php: needs merge d:\git_files\MyIntervals\PHP-CSS-Parser>git add Size.php fatal: pathspec 'Size.php' did not match any files d:\git_files\MyIntervals\PHP-CSS-Parser>git add *.php d:\git_files\MyIntervals\PHP-CSS-Parser>git rebase --continue [detached HEAD 7039532] Add support for dvh, svh, lvh. Close #412 Author: Kai König <50620424+KaiOnGitHub@users.noreply.github.com> 1 file changed, 17 insertions(+), 1 deletion(-) Auto-merging src/Value/Size.php CONFLICT (content): Merge conflict in src/Value/Size.php error: could not apply bc058a4... fix 120 char per line limit hint: Resolve all conflicts manually, mark them as resolved with hint: "git add/rm <conflicted_files>", then run "git rebase --continue". hint: You can instead skip this commit: run "git rebase --skip". hint: To abort and get back to the state before "git rebase", run "git rebase --abort". Could not apply bc058a4... fix 120 char per line limit d:\git_files\MyIntervals\PHP-CSS-Parser>git push -f fatal: You are not currently on a branch. To push the history leading to the current (detached HEAD) state now, use git push origin HEAD:<name-of-remote-branch> d:\git_files\MyIntervals\PHP-CSS-Parser>git push origin HEAD:pr/415 error: The destination you provided is not a full refname (i.e., starting with "refs/"). We tried to guess what you meant by: - Looking for a ref that matches 'pr/415' on the remote side. - Checking if the <src> being pushed ('HEAD') is a ref in "refs/{heads,tags}/". If so we add a corresponding refs/{heads,tags}/ prefix on the remote side. Neither worked, so we gave up. You must fully qualify the ref. hint: The <src> part of the refspec is a commit object. hint: Did you mean to create a new branch by pushing to hint: 'HEAD:refs/heads/pr/415'? error: failed to push some refs to 'https://github.com/MyIntervals/PHP-CSS-Parser.git' d:\git_files\MyIntervals\PHP-CSS-Parser>git push origin HEAD:refs/pull/415/head Enumerating objects: 9, done. Counting objects: 100% (9/9), done. Delta compression using up to 8 threads Compressing objects: 100% (5/5), done. Writing objects: 100% (5/5), 576 bytes | 576.00 KiB/s, done. Total 5 (delta 4), reused 0 (delta 0), pack-reused 0 remote: Resolving deltas: 100% (4/4), completed with 4 local objects. To https://github.com/MyIntervals/PHP-CSS-Parser.git ! [remote rejected] HEAD -> refs/pull/415/head (deny updating a hidden ref) error: failed to push some refs to 'https://github.com/MyIntervals/PHP-CSS-Parser.git' d:\git_files\MyIntervals\PHP-CSS-Parser>git add *.php d:\git_files\MyIntervals\PHP-CSS-Parser>git rebase --continue Successfully rebased and updated refs/heads/pr/415. d:\git_files\MyIntervals\PHP-CSS-Parser>git push -f fatal: The upstream branch of your current branch does not match the name of your current branch. To push to the upstream branch on the remote, use git push origin HEAD:main To push to the branch of the same name on the remote, use git push origin HEAD To choose either option permanently, see push.default in 'git help config'. To avoid automatically configuring an upstream branch when its name won't match the local branch, see option 'simple' of branch.autoSetupMerge in 'git help config'. d:\git_files\MyIntervals\PHP-CSS-Parser>git push origin HEAD Enumerating objects: 9, done. Counting objects: 100% (9/9), done. Delta compression using up to 8 threads Compressing objects: 100% (5/5), done. Writing objects: 100% (5/5), 576 bytes | 576.00 KiB/s, done. Total 5 (delta 4), reused 0 (delta 0), pack-reused 0 remote: Resolving deltas: 100% (4/4), completed with 4 local objects. remote: remote: Create a pull request for 'pr/415' on GitHub by visiting: remote: https://github.com/MyIntervals/PHP-CSS-Parser/pull/new/pr/415 remote: To https://github.com/MyIntervals/PHP-CSS-Parser.git * [new branch] HEAD -> pr/415 |
Step 1: From your project repository, check out a new branch and test the changes. git checkout -b KaiOnGitHub-patch-1 main git pull https://github.com/KaiOnGitHub/PHP-CSS-Parser.git patch-1 Step 2: Merge the changes and update on GitHub. git checkout main git merge --no-ff KaiOnGitHub-patch-1 git push origin main Trying this now. |
I can see this is totally stupid. It will clearly merge the changes into Much simpler to just start over. Avoids a waste of time with systems that don't work or cannot be understood. |
@JakeQZ Maybe the GitHub documentation for working on a PR branch of a fork is helpful: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork (I have to admit that I have never tries this myself.) |
Yes, if the PR submitter has enabled “allow contributions from maintainers” setting, you’ll be able to commit to their forked branch. However, I find that, for simple merge conflicts, the web-based UI GitHub provides is usually sufficient (I used it in this case). |
For now, the `TestCase` just tests that all the unit values are parsed. (Other tests can be added here, but are beyond the scope of this change.)
I think I maybe went wrong by fetching the remote PR into my local repo. Didn't have a problem after cloning their fork into a separate directory. |
Co-authored-by: Oliver Klee <github@oliverklee.de>
Co-authored-by: Oliver Klee <github@oliverklee.de>
dvh
, lvh
and svh
length units
Fixes #412