Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DomCrawler] Added support for slicing nodes#10699
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Remove@api please. It can't be part of the stable api untill it... well, stablisies ;)
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.
Of cource, I'll do. Thanks for understanding I'm apprentice. :)
jakzal commentedApr 13, 2014
Would be nice to cover the new method by tests andthe PR checklist is missing. |
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.
Why not simply call:
returnnewstatic(iterator_to_array(new \LimitIterator($this,$offset,$length)),$this->uri);
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.
Oh, you right! Will be used in next commit.
beratdogan commentedApr 13, 2014
@jakzal I don't have right experience on writing tests. Could you help to me on this? |
jakzal commentedApr 13, 2014
@Grafitus look at the existing ones in the CrawlerTest. If you still need help, I can write tests for you. |
beratdogan commentedApr 14, 2014
I've problems with Git. I wrote the first commit on GitHub --Editing Files. Now, I tried to clone my forked repository into my PC. I couldn't see my 'patch-1' branch, and my changes. :/ |
jakzal commentedApr 14, 2014
@Grafitus it is there:https://github.com/grafitus/symfony/tree/patch-1 Run: git pullgit checout patch-1 |
beratdogan commentedApr 16, 2014
OMG! I think I made it! 👍 @jakzal can check my commits please? :) |
cordoval commentedApr 17, 2014
you changed permissions on the files not good 👶 |
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.
Should beint
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.
@yguedidi Used asinteger in doc blocks, likeeq() andgetNode(). Are you sure?
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.
Yes, look at#10701 ;)
beratdogan commentedApr 18, 2014
@cordoval You got me! :) I'm still new at Unix-like systems. And file permissions makes me nervous. Do you have suggestions for me to cope with permissions? |
cordoval commentedApr 18, 2014
there is an option on .git/config to avoid making that mistake, also there are tools to fix cs like php-cs-fixer and if you need help doing a PR check the documentation for how the table PR is defined. You can finally use tools like Gush to send a PR also. |
beratdogan commentedMay 24, 2014
@jakzal Everything looks good. Can you check for merging? |
stof commentedMay 24, 2014
@beratdogan there is 2 things preventing the merge:
|
beratdogan commentedMay 24, 2014
I guess there is no anything I can do, right? |
stof commentedMay 24, 2014
for the first part, no. |
fabpot commentedJun 3, 2014
Can you rebase this PR on current master? |
fabpot commentedJun 3, 2014
Thank you@beratdogan. |
There are no easy way to slice nodes in specific range. I created a method using \LimitIterator. Works fine for me.
An example using is: