search

davetron5000 / gli

Make awesome command-line applications the easy way
http://davetron5000.github.io/gli
Apache License 2.0
1.26k stars 102 forks source link

Extending global scope causes wild behavior #275

Closed dblock closed 6 years ago

dblock commented 6 years ago

Coming from https://github.com/rmosolgo/graphql-ruby/issues/1775. I might not understand the issue completely.

The documentation suggests to include GLI::App in global scope. This caused the graphql-ruby gem to call GLI's arguments method as I believe it extends Object (still debugging this one). An easy workaround is to place the GLI code into a class as seen here.

class App
  extend GLI::App

  # use normally
end

exit App.run(ARGV)

Should this be the recommended way of doing things?

davetron5000 commented 6 years ago

That is the solution when there is another library that is polluting global scope. It could be changed by modifying the mk_binfile method in scaffold.rb (and requisite docs).

johnmcdowall commented 5 years ago

FWIW I just ran into this, and pretty sure I wasn't polluting the global scope. I was calling .version on an OpenStruct that had a version key, and instead somehow it ended up calling what looked like the GLI version directive, and therefore crashing.

I got around it by doing as OP suggested and making the GLI::App extend inside a dummy class.

davetron5000 commented 5 years ago

If you included GLI::App at the top level of your main file, then all of GLI's methods get defined there, which is on Object. That means that in this case, the method version got added to Object and since OpenStruct inherits from object, it will have that method defined. Not sure how OpenStruct is implemented, but it's likely that it will not override a method that's already defined, so it sees version defined and thus doesn't define its own for the data inside itself.

johnmcdowall commented 5 years ago

Yeah that's what I suspected. I was just following the GLI docs website to get up and running at first, so maybe that needs to be updated to show using the dummy class instead, so people don't run into this issue out the gate. I mean it all makes sense if you sit and think it through, but when you're trying to get something done quickly the lizard brain is happy to see and do :)

Thanks for GLI!

johnmcdowall commented 5 years ago

PS. Happy to submit PR to the pages branch with that kind of update if you think its appropriate

dblock commented 5 years ago

That was in https://github.com/davetron5000/gli/pull/278/files, no? Maybe the website needs to be pushed?

johnmcdowall commented 5 years ago

Yeah, http://davetron5000.github.io/gli/ doesn't show those changes.

davetron5000 commented 5 years ago

Ahhhh! Yeah, I've now updated the gh-pages for this so the example shows using a namespacve.