Skip to content
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

Selectorlist extract first #624

Closed
wants to merge 7 commits into from

Conversation

@ananana
Copy link
Contributor

@ananana ananana commented Mar 4, 2014

Fixed failed tests in pull request #572 that adds extract_first() method to SelectorList; related to original issue #568

golewski and others added 7 commits Jan 30, 2014
…y into selectorlist-extract-first
@shaneaevans
Copy link
Member

@shaneaevans shaneaevans commented Mar 5, 2014

code looks ok 👍

@@ -169,9 +169,17 @@ def css(self, xpath):
def re(self, regex):
return flatten([x.re(regex) for x in self])

def re_first(self, regex):
for el in iflatten((x.re(regex) for x in self)):

This comment has been minimized.

@kmike

kmike Apr 15, 2014
Member

iflatten(x.re(regex) for x in self) should also work

@curita
Copy link
Member

@curita curita commented Mar 13, 2015

@ananana hi! Sorry for such late follow up, we're planning on (finally) getting this pull request merged.

There are some tasks still needed for doing so.
First, we should rebase current changes from scrapy:master. I tried to do it but there were some conflicts, maybe it's best to start a new pull requests and cherry-pick commits from this pull request.

Second, last commit (0587132) is a merge, it'd be best if it wasn't, we use merge commits for merging pull requests, so it's preferred that all commits from a given pull request are simple commits.

Are you interested in making these changes? Let us know, we can implement them if that's not the case.

curita added a commit that referenced this pull request Mar 19, 2015
@curita
Copy link
Member

@curita curita commented Mar 19, 2015

Merged in ff64584

@curita curita closed this Mar 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants