Enable irb flags for local geordi config and prioritize irb flags from config above default behavior #239

brunosedler · 2025-08-07T11:57:44Z

No description provided.

codener

Hi Bruno, thanks for this PR. The code is alright, but I'd like to change one detail plus have tests restructured a bit. Could you have a look?

codener · 2025-08-18T13:08:16Z

features/console.feature

+    """
+  When I run `geordi console staging`
+  Then the output should contain "# Opening a Rails console on staging"
+    And the output should contain "Util.run! ssh, [email protected], -t, cd /var/www/example.com/current && bash --login -c 'bundle exec rails console -e staging -- --foo --baz"


Please explicitly expect to not see "nomultiline" as in the scenarios above. Otherwise it might have been printed right after --baz. Same below.

codener · 2025-08-18T13:18:56Z

features/console.feature

+    And the output should contain "Util.run! ssh, [email protected], -t, cd /var/www/example.com/current && bash --login -c 'bundle exec rails console -e staging -- --foo --baz"
+
+
+Scenario: Opening a remote Rails console with an irb version >= 1.2.0, a Ruby version < 3.0 and preconfigured irb flags from local and global settings


It is really important to choose scenario titles not to repeat the text below, but such that they convey their background and intention. The current scenario titles are hardly graspable.

Lets make the scenario titles clearer. I'd like them like this:

4: IRB < 1.2.0 has no pasting issues and thus needs no --nomultiline switch

13: IRB on Ruby 3+ has no pasting issues and thus needs no --nomultiline switch

21: IRB 1.2.0+ on Ruby <3 is horribly slow when pasting long strings – we work around this by automatically setting the --nomultiline switch

30: Opening a remote Rails console with a slow pasting IRB sets the --nomultiline switch

48: Opening a remote Rails console with a slow pasting IRB and IRB flags from the global settings does not set the --nomultiline switch

66: IRB flags in local Geordi settings override global and automatic IRB flags

85: Empty IRB flags in the local Geordi settings also override global IRB flags

Well, I just realized that most of these cases should be unit tests of Util#console_command. Let's only preserve these integration tests:

4: Opening a local Rails console

21: IRB 1.2.0+ on Ruby <3 is horribly slow when pasting long strings – we work around this by automatically setting the --nomultiline switch

30: Opening a remote Rails console with a slow pasting IRB sets the --nomultiline switch

85: Empty IRB flags in the local Geordi settings override global and automatic IRB flags

All others should be turned into unit tests of Util#console_command.

codener · 2025-08-18T13:19:16Z

features/console.feature

+    And the output should contain "Util.run! ssh, [email protected], -t, cd /var/www/example.com/current && bash --login -c 'bundle exec rails console -e staging -- --foo --baz"
+
+
+Scenario: Opening a remote Rails console with an irb version >= 1.2.0, a Ruby version < 3.0 and empty irb flags in local settings and preconfigured irb flags in global settings


Unless it's hard to implement, this should be a unit test, as it is an edge case of the scenario above.

codener · 2025-08-18T13:21:44Z

features/console.feature

  When I run `geordi console staging`
  Then the output should contain "# Opening a Rails console on staging"
-    And the output should contain "Util.run! ssh, [email protected], -t, cd /var/www/example.com/current && bash --login -c 'bundle exec rails console -e staging -- --nomultiline --foo --baz"
+    And the output should contain "Util.run! ssh, [email protected], -t, cd /var/www/example.com/current && bash --login -c 'bundle exec rails console -e staging -- --foo --baz"


Did we settle that global IRB flags should kill the nomultiline switch? I'm afraid this might be impractical.

Could we add the nomultiline switch to flags taken from the global config, and only let locale config override that?

Since we're still supporting Ruby 2.7, we cannot yet get rid of all this code.

Sorry, I guess there was a misunderstand in that case, I thought that was what we settled on.

However, IMHO I think we should use the global irb_flags, if they have been configured. If people have set global irb flags, they have done so intentionally and should know their flags and should be able to set the --no-multiline if they need to. If they don't care enough to set irb_flags, they'll still get the --no-multiline option. For me, it would be kind of unexpected if options work differently between local and global settings

We discussed and agreed to merge the automatic --nomultiline switch with flags taken from the global config, and only let locale config override that. It's an extra piece of complexity, but the best solution we found.

codener · 2025-08-18T13:23:16Z

lib/geordi/util.rb

+            irb_flags_from_config
+          elsif irb_version >= Gem::Version.new('1.2') && ruby_version < Gem::Version.new('3.0')
            Interaction.note 'Using --nomultiline switch for faster pasting'
            '--nomultiline'


Maybe you could pass this to Settings#irb_flags as "extra" flag, and only return it with the global flags?

spec/spec_helper.rb

codener · 2025-08-18T13:34:18Z

spec/settings_spec.rb

  end
+
+
+  describe '#irb_flags', type: :aruba do


After updating the other tests, please remove example in this file that have been covered by other tests.

codener

Thanks for pushing this through!

codener · 2025-08-29T10:02:02Z

lib/geordi/settings.rb

+      return [@local_settings['irb_flags'].to_s, :local] if @local_settings.key? 'irb_flags'
+      [@global_settings['irb_flags'].to_s, :global] if @global_settings.key? 'irb_flags'


Would be more idiomatic as:

Suggested change

return [@local_settings['irb_flags'].to_s, :local] if @local_settings.key? 'irb_flags'

[@global_settings['irb_flags'].to_s, :global] if @global_settings.key? 'irb_flags'

if @local_settings.key? 'irb_flags'

[@local_settings['irb_flags'].to_s, :local]

elsif @global_settings.key? 'irb_flags'

[@global_settings['irb_flags'].to_s, :global]

end

brunosedler requested a review from codener August 7, 2025 12:00

brunosedler force-pushed the bs/use-irb-flags-from-local-geordi-config branch from 6c1e956 to bf151f0 Compare August 7, 2025 12:26

codener requested changes Aug 18, 2025

View reviewed changes

brunosedler requested a review from codener August 29, 2025 07:28

codener approved these changes Aug 29, 2025

View reviewed changes

Respect irb flags from local geordi config

3ff6547

brunosedler force-pushed the bs/use-irb-flags-from-local-geordi-config branch from 4510456 to 3ff6547 Compare August 29, 2025 12:24

brunosedler merged commit 3ff6547 into master Aug 29, 2025
4 checks passed

brunosedler deleted the bs/use-irb-flags-from-local-geordi-config branch August 29, 2025 12:25

		And the output should contain "Util.run! ssh, [email protected], -t, cd /var/www/example.com/current && bash --login -c 'bundle exec rails console -e staging -- --foo --baz"


		Scenario: Opening a remote Rails console with an irb version >= 1.2.0, a Ruby version < 3.0 and preconfigured irb flags from local and global settings

		return [@local_settings['irb_flags'].to_s, :local] if @local_settings.key? 'irb_flags'
		[@global_settings['irb_flags'].to_s, :global] if @global_settings.key? 'irb_flags'

Enable irb flags for local geordi config and prioritize irb flags from config above default behavior #239

Enable irb flags for local geordi config and prioritize irb flags from config above default behavior #239

Uh oh!

Conversation

brunosedler commented Aug 7, 2025

Uh oh!

codener left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codener left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants