You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Summary: let collection help work with disabled collection configuration and return a clear failure for commands that require configuration, instead of raising from a missing path.
Verification: focused baseline/fixed reproduction, combined RBS 4.2.0 consumer models, and RuboCop (738 files, zero offenses). No tests are added in this PR.
Compatibility: no public API removal or dependency/version change.
Thank you for the patch, and sorry for the slow reply. I am going to close this one.
You are right that the current code is bad: rbs --no-collection collection install dies with a bare RuntimeError carrying an empty message, and I reproduced that. But I do not think the combination is worth supporting.
--no-collection is not an option of the collection subcommand. It belongs to LibraryOptions, and its only job is to make LibraryOptions#loader skip add_collection, so that rbs list, rbs ast, rbs validate and friends can build an environment without pulling in the gem RBS collection. run_collection only sees the nil because every subcommand is handed the same options object. --no-collection and the config_path = options.config_path or raise line were in fact added in the same commit (38fd8b2, the one that introduced rbs collection), so combining the two was never something we meant to support. Asking rbs collection to run while telling rbs to ignore the collection configuration does not describe anything a user wants.
On the help branch specifically: I do not see what it buys us. rbs collection help already works without --no-collection, and nobody is going to pass --no-collection in order to read the help text. Lifting one branch out of the case to serve that combination costs more in readability than the behaviour is worth.
So we would be adding code, and a user-facing error message, for an invocation that has no meaning. I would rather leave the surface as it is.
If you have run into this for real -- a wrapper or an alias that always passes --no-collection, say -- please tell me. A concrete case would change my mind, and then I would want the fix to be just the nil check, without the help special case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary: let collection help work with disabled collection configuration and return a clear failure for commands that require configuration, instead of raising from a missing path.
Verification: focused baseline/fixed reproduction, combined RBS 4.2.0 consumer models, and RuboCop (738 files, zero offenses). No tests are added in this PR.
Compatibility: no public API removal or dependency/version change.