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

Avoid setting fixed app name in console prompt #51732

Merged
merged 1 commit into from May 14, 2024

Conversation

st0012
Copy link
Contributor

@st0012 st0012 commented May 3, 2024

Motivation / Background

IRB allows changing its prompt's name during the session, like:

irb(main):001> conf.irb_name = "foo"
=> "foo"
foo(main):002> 

But because I previously set the app name as part of the prompt, instead of via the irb_name config, such change can't happen in a Rails console:

Loading development environment (Rails 7.2.0.alpha)
test-app(dev)> conf.irb_name = "foo"
=> "foo"
test-app(dev)> 

This can be problematic as tools/features can rely on the name change to indicate state change (example).

Detail

Instead of setting app name as a fixed value in prompt, I replaced it with %N, which IRB uses to display the irb_name attribute. And then we can set assign the app name to IRB.conf[:IRB_NAME] (default is "irb").

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the railties label May 3, 2024
Copy link
Member

@flavorjones flavorjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Can you squash these commits?

@st0012 st0012 force-pushed the fix-rails-console-prompt branch from 6351533 to a507f53 Compare May 6, 2024 17:11
@st0012 st0012 requested a review from flavorjones May 6, 2024 17:11
Instead, the app's name should be set via a separate IRB config, which
will allow it to be changed at runtime.

Co-authored-by: Mike Dalessio <[email protected]>
@st0012 st0012 force-pushed the fix-rails-console-prompt branch from a507f53 to beba30b Compare May 6, 2024 23:02
@flavorjones flavorjones added the ready PRs ready to merge label May 8, 2024
@carlosantoniodasilva carlosantoniodasilva merged commit 5e3611e into rails:main May 14, 2024
4 checks passed
carlosantoniodasilva added a commit that referenced this pull request May 14, 2024
Avoid setting fixed app name in console prompt
@rafaelfranca
Copy link
Member

Test seems to be failing now because of this change

@rafaelfranca rafaelfranca deleted the fix-rails-console-prompt branch May 14, 2024 23:09
carlosantoniodasilva added a commit that referenced this pull request May 14, 2024
We need to pass the verbose option in order to test the full expected
output. Not sure why these were green on the branch.

Introduced by #51732.
carlosantoniodasilva added a commit that referenced this pull request May 15, 2024
We need to pass the verbose option in order to test the full expected
output. Not sure why these were green on the branch.

Introduced by #51732.
@st0012
Copy link
Contributor Author

st0012 commented May 15, 2024

Sorry for causing the failure. It's likely caused by the latest IRB release and was not caught here because the branch hadn't bumped its version yet.

@p8 p8 mentioned this pull request May 15, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
railties ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants