-
Notifications
You must be signed in to change notification settings - Fork 144
[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