bridgetownrb / bridgetown

A next-generation progressive site generator & fullstack framework, powered by Ruby
https://www.bridgetownrb.com
MIT License
1.11k stars 114 forks source link

Move site and collections console commands to locals #853

Closed mpace965 closed 3 months ago

mpace965 commented 4 months ago

This is a 🐛 bug fix.

Summary

Move the site and collections console commands to be local variables in the IRB shell.

Context

Console methods have precedence over Ruby methods with the same name. This means that any method name or local variable will be clobbered by the IRB commands site and collections. This presents an issue when using the IRB or Pry object navigation features, e.g.

3.0.6 :001 > cws site
irb: warn: can't alias source from irb_source.
#<Bridgetown::Site >
3.0.6 :002 > render
/Users/mattpace/.rvm/gems/ruby-3.0.6/gems/activesupport-7.1.2/lib/active_support/isolated_execution_state.rb:70:in `state': stack level too deep (SystemStackError)
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/gems/activesupport-7.1.2/lib/active_support/isolated_execution_state.rb:38:in `[]'
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/gems/activesupport-7.1.2/lib/active_support/current_attributes.rb:173:in `current_instances'
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/gems/activesupport-7.1.2/lib/active_support/current_attributes.rb:100:in `instance'
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/gems/activesupport-7.1.2/lib/active_support/current_attributes.rb:127:in `sites'
        from /Users/mattpace/dev/mpace965/bridgetown/bridgetown-core/lib/bridgetown-core/current.rb:11:in `site'
        from /Users/mattpace/dev/mpace965/bridgetown/bridgetown-core/lib/bridgetown-core/commands/console.rb:6:in `site'
        from /Users/mattpace/dev/mpace965/bridgetown/bridgetown-core/lib/bridgetown-core/commands/console.rb:10:in `collections'
        from /Users/mattpace/dev/mpace965/bridgetown/bridgetown-core/lib/bridgetown-core/commands/console.rb:10:in `collections'
         ... 11890 levels...
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/gems/bundler-2.5.3/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/gems/bundler-2.5.3/exe/bundle:20:in `<top (required)>'
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/bin/bundle:23:in `load'
        from /Users/mattpace/.rvm/gems/ruby-3.0.6/bin/bundle:23:in `<main>'

In this example, I've tried to execute Bridgetown::Site#render after setting the IRB workspace to site. This results in a SystemStackError because when calling collections on this line, the collections IRB command is executed instead of the collections instance method on site.

This PR moves site and collections to local variables that are passed into the main IRB workspace as a binding. This does mean that over the course of an IRB session, a user is free to initialize site or collections to something else. The upshot is that there's no longer a risk of clobbering these method names.

jaredcwhite commented 3 months ago

TIL. Thanks for the PR @mpace965!

jaredcwhite commented 3 months ago

@mpace965 I ended up hunting around for yet a third option to facilitate this, as the previous method of setting the IRB WorkSpace binding was making the prompt in the console look really weird (it was a long Bridgetown classname instead of main). Take a look at my fix here: https://github.com/bridgetownrb/bridgetown/commit/dc08da22084dee5dc5d4d828d0c0e171ce0ec0c4

It seems like that still facilitates your example of using cws but if you think there's any issue let me know. Thanks!

mpace965 commented 3 months ago

If you were trying out the cws command in IRB, that is probably what was causing the Bridgetown class name to show up instead of main. That is actually native functionality of IRB, unrelated to this change. That part of the prompt indicates the current workspace (which the cws command changes).

That being said, it looks like defining the methods that way works just as well! Thanks for taking a look.