Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Search links and images through css or xpath #287

Merged
merged 3 commits into from Feb 9, 2013

Conversation

Projects
None yet
3 participants
Contributor

philou commented Feb 2, 2013

Thanks for mechanize, it's a great gem. I have been missing the following search methods though.

Owner

leejarvis commented Feb 2, 2013

I'm not sure I see the benefit to this when you can simply do

page.links_with(:class => 'letter')
page.images_with(:class => 'pretty')
Owner

drbrain commented Feb 2, 2013

Agreed, seems like duplicate functionality.

What prevents the user from using a selector that doesn't select a link? This would lead to unintuitive behavior. (The same is true for links_with, but it is long-standing.)

Contributor

philou commented Feb 3, 2013

Hi,

I'm actually using it with more complex xpathes or css selectors (css
specifying sub selectors and using commas), so I did have the need. Maybe i
could make it clearer from the tests.

I though of the fact that the selector could select something else than à
link. Would a filter throwing exception be better ?

Thanks
Philippe
Le 2 févr. 2013 22:35, "Eric Hodel" notifications@github.com a écrit :

Agreed, seems like duplicate functionality.

What prevents the user from using a selector that doesn't select a link?
This would lead to unintuitive behavior. (The same is true for links_with,
but it is long-standing.)


Reply to this email directly or view it on GitHubhttps://github.com/sparklemotion/mechanize/pull/287#issuecomment-13037830.

Owner

leejarvis commented Feb 3, 2013

I'm unsure about this for two reasons:

  1. Adding complexity to the public API
  2. As Eric says, ambiguity regarding element selection.

I'm interested to see some code in which you would utilise this feature.
Maybe there are alternatives we're missing.

On 3 Feb 2013, at 08:02, Philippe Bourgau notifications@github.com wrote:

Hi,

I'm actually using it with more complex xpathes or css selectors (css
specifying sub selectors and using commas), so I did have the need. Maybe i
could make it clearer from the tests.

I though of the fact that the selector could select something else than à
link. Would a filter throwing exception be better ?

Thanks
Philippe
Le 2 févr. 2013 22:35, "Eric Hodel" notifications@github.com a écrit :

Agreed, seems like duplicate functionality.

What prevents the user from using a selector that doesn't select a link?
This would lead to unintuitive behavior. (The same is true for
links_with,
but it is long-standing.)


Reply to this email directly or view it on GitHub<
https://github.com/sparklemotion/mechanize/pull/287#issuecomment-13037830>.


Reply to this email directly or view it on
GitHubhttps://github.com/sparklemotion/mechanize/pull/287#issuecomment-13043822.

Owner

drbrain commented Feb 3, 2013

I also think you can use :xpath with links_with which gives identical functionality.

Contributor

philou commented Feb 4, 2013

@drbrain : I tried it, but both links_with(css: ...) or links_with(xpath: ...) don't work because :

  • the methods css or xpath don't exist
  • they can be many xpathes or css for a single dom element, so we'd have to review the current implementation or make some special cases to support it

@injekt : for an example, suppose you are scrapping this page http://www.auchandirect.fr and that you want to get all the links with css '#footer-menu a' (links in the bottom grey pane). I cannot think of any other way to get them all.

Owner

leejarvis commented Feb 4, 2013

Yeah it looks like the :xpath option doesn't work. I would however prefer that over adding new methods to the public API, but I don't think it'll be an easy change, and it doesn't solve the problem with ambiguous elements being handed to the method, but perhaps we raise an exception like you suggested.

Owner

leejarvis commented Feb 5, 2013

I'm not sure about :selector I realize xpath and css can be used interchangeably but I think I prefer :css and :xpath options or maybe just :search I'll let @drbrain make the final decision on this, though

Owner

leejarvis commented Feb 8, 2013

Ok we already use search as a method on Mechanize::Page which is delegated to our Nokogiri document so perhaps the key should be :search. @philou if you could rename :selector to :search and remove the normalize method (you can still wrap the criteria in a Hash) then I will merge this. Cheers!

@leejarvis leejarvis added a commit that referenced this pull request Feb 9, 2013

@leejarvis leejarvis Merge pull request #287 from philou/master
Search links and images through css or xpath
e0ac874

@leejarvis leejarvis merged commit e0ac874 into sparklemotion:master Feb 9, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment