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

Replace Hpricot with Nokogiri, since Hpricot is officially dead. shoes/s... #190

Merged
merged 1 commit into from Jan 28, 2013
Merged

Replace Hpricot with Nokogiri, since Hpricot is officially dead. shoes/s... #190

merged 1 commit into from Jan 28, 2013

Conversation

plexus
Copy link
Member

@plexus plexus commented Jan 20, 2013

Replace Hpricot with Nokogiri, since Hpricot is officially dead. #176

Besides lib/shoes/help.rb I also updated samples/expert-funnies and the Japanese
manual since they both referred to using Hpricot, however they both depend on 'download'
which seems to be (currently) unimplemented.

@plexus
Copy link
Member Author

plexus commented Jan 20, 2013

I squashed my commits and I'd say this can go in now. Like I said before there are no unit tests for this yet, but I'd like to address that while factoring out some aspects of the manual generation in their own files/classes, since the current lib/shoes/help and lib/shoes/manual are not that easily testable.

@steveklabnik
Copy link
Member

🤘

This says it can't be automatically merged though. Can you rebase off of current master?

@PragTob
Copy link
Member

PragTob commented Jan 20, 2013

👍
Yeah please rebase against the current master or merge it if you want to :-)

Thanks + Cheers,
Tobi

Besides lib/shoes/help.rb I also updated samples/expert-funnies and the Japanese
manual since they both referred to using Hpricot, however they both depend on 'download'
which seems to be (currently) unimplemented.
@plexus
Copy link
Member Author

plexus commented Jan 21, 2013

😎

Rebased!

@steveklabnik
Copy link
Member

  1) Shoes::Swt::Package::Jar when creating a .jar creates .jar smaller than 50MB
2637     Failure/Error: File.size(output_file).should be < 50 * 1024 * 1024
2638       expected: < 52428800
2639            got:   52736411
2640     # ./spec/swt_shoes/package_jar_spec.rb:34:in `(root)'

:'(

@wasnotrice
Copy link
Member

That's an arbitrary number. Will be less when we pull out packaging artifacts. Guess that should happen soon :/

@PragTob
Copy link
Member

PragTob commented Jan 21, 2013

Yeah probably, I'll increase the limit for now and then merge this :-)

@PragTob
Copy link
Member

PragTob commented Jan 21, 2013

Okay people, am I doing something wrong here? For me, both on current master and on this PR I get the following error when running the manual (sample99.rb)...

tobi@speedy:~/github/shoes4$ bin/shoes samples/sample99.rb 
WARNING: Global access to Rake DSL methods is deprecated.  Please include
    ...  Rake::DSL into classes and modules which use the Rake DSL methods.
WARNING: DSL method Manual#link called at /home/tobi/github/shoes4/lib/shoes/help.rb:61:in `table_of_contents'
SystemStackError: stack level too deep
        __send__ at org/jruby/RubyBasicObject.java:1673
            send at org/jruby/RubyKernel.java:2081
  method_missing at /home/tobi/github/shoes4/lib/shoes/url.rb:12
        __send__ at org/jruby/RubyBasicObject.java:1673
            send at org/jruby/RubyKernel.java:2081
  method_missing at /home/tobi/github/shoes4/lib/shoes/url.rb:12
        __send__ at org/jruby/RubyBasicObject.java:1673
            send at org/jruby/RubyKernel.java:2081
  method_missing at /home/tobi/github/shoes4/lib/shoes/url.rb:12

No time to investigate further... -sorry :o

@davorb
Copy link
Member

davorb commented Jan 22, 2013

I can just say that I'm not getting that error on OS X / JRuby 1.7.1.

@ashbb ashbb merged commit 630d689 into shoes:master Jan 28, 2013
@ashbb
Copy link
Member

ashbb commented Jan 28, 2013

@arnebrasseur Thanks for the great work! We could migrate from Hpricot to Nokogiri. :)

@plexus
Copy link
Member Author

plexus commented Jan 28, 2013

My pleasure! I hope to make some more time for Shoes in the near future.

On 28 January 2013 14:15, ashbb notifications@github.com wrote:

@arnebrasseur https://github.com/arnebrasseur Thanks for the great
work! We could migrate from Hpricot to Nokogiri. :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/190#issuecomment-12781024.

@sferik
Copy link

sferik commented Jan 29, 2013

Why not migrate to MultiXML so you're not coupled to any particular XML library?

@steveklabnik
Copy link
Member

We're on JRuby, so we want Hpricot's baller implementation in Java.

@sferik
Copy link

sferik commented Jan 29, 2013

Fair enough. But just to be clear, MultiXML allows you to continue using Nokogiri’s implementation under the hood. It just means you don't need to change all your interfaces if you ever want to switch again in the future (say, if there's an even better implementation, or if you move off JRuby, or if the maintainers of Nokogiri suddenly vanish).

@steveklabnik
Copy link
Member

Totes. Since we bundle everything up as a download, we want to keep things as slim as possible, and only introduce extra deps when we have to. Good to know for the future though 👍

@PragTob
Copy link
Member

PragTob commented Jan 31, 2013

I think that would actually be a very good idea since we want to support multiple backends, potentially an MRI/Qt one so if the change isn't too big it would be cool :-)

@plexus
Copy link
Member Author

plexus commented Jan 31, 2013

Important detail here : we're mostly using Nokogiri for generating
XML, not for parsing it. The only place where it's used for parsing is
in a few examples, and a single mention in the Japanese manual.

From the description of MultiXML : "A generic swappable back-end for
XML parsing"

@PragTob
Copy link
Member

PragTob commented Jan 31, 2013

Thanks for that input didn't look at it that closely!

@steveklabnik
Copy link
Member

@arnebrasseur currently no, but there are methods that parse XML in the shoes API, IIRC. The stuff that fetches websites parses the sites too.

@plexus plexus deleted the nokigiri_squash branch February 14, 2013 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants