Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Introduce exit! command #851

Merged
merged 7 commits into from
Feb 10, 2024
Merged

Conversation

ignacio-chiazzo
Copy link
Contributor

@ignacio-chiazzo ignacio-chiazzo commented Jan 31, 2024

Problem

Currently, exit! does not save the history of the console' input like the following example

irb(main):001:0> a = 1
=> 1
irb(main):002:0> b = 2
=> 2
irb(main):003:0> exit

Open another session and type up-arrow to check the above history
irb(main):001:0> b = 2 # (history is saved!)

If we run the same for exit!, the history isn't saved.
Closes #612

Solution

I extended the command exit! and used a new class ExitForcedAction. Note that we can't use Exit! as the class name since ! isn't allowed.

This will call the irb_exit! method, which will throw the same exception as exit (throw :IRB_EXIT) and the main method run catches it and calls `save_history``.

Demo

In the following example, I test that the history is saved and Kernel.exit! exits the console even when the session is nested.

Screen.Recording.2024-02-02.at.2.36.03.PM.mov

@st0012
Copy link
Member

st0012 commented Jan 31, 2024

Calling exit! now behaves differently then exit:

exit

$ be ruby test.rb

From: test.rb @ line 1 :

 => 1: binding.irb
    2: binding.irb

irb(main):001> exit

From: test.rb @ line 2 :

    1: binding.irb
 => 2: binding.irb

irb(main):001> exit

exit!

$ be ruby test.rb

From: test.rb @ line 1 :

 => 1: binding.irb
    2: binding.irb

irb(main):001> exit! # the process exists right away

I think the solution should also maintain this behaviour while saving the history.

@ignacio-chiazzo ignacio-chiazzo changed the title Fix history on exit! Save history when calling Kernel.exit! Feb 2, 2024
lib/irb.rb Outdated Show resolved Hide resolved
lib/irb/cmd/exit_forced_action.rb Outdated Show resolved Hide resolved
@ignacio-chiazzo ignacio-chiazzo force-pushed the fix-history-on-exit! branch 4 times, most recently from 6f0843f to 77146ff Compare February 2, 2024 21:01
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

I think we're pretty close, just want to clarify 2 changes.

lib/irb.rb Outdated Show resolved Hide resolved
lib/irb.rb Show resolved Hide resolved
lib/irb/cmd/exit_forced_action.rb Outdated Show resolved Hide resolved
@st0012 st0012 added the bug Something isn't working label Feb 2, 2024
@st0012 st0012 added enhancement New feature or request and removed bug Something isn't working labels Feb 7, 2024
@st0012 st0012 changed the title Save history when calling Kernel.exit! Introduce exit! command Feb 7, 2024
lib/irb.rb Outdated Show resolved Hide resolved
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR ❤️

@st0012 st0012 merged commit c0a5f31 into ruby:master Feb 10, 2024
29 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 10, 2024
(ruby/irb#851)

* Added failing test for when writing history on exit

* Save history on exit

* Exit early when calling Kernel.exit

* use status 0 for kernel.exit

* Added test for nested sessions

* Update lib/irb.rb

---------

ruby/irb@c0a5f31679

Co-authored-by: Stan Lo <stan001212@gmail.com>
@nobu
Copy link
Member

nobu commented Feb 11, 2024

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

IRB doesn't save input history when exited with exit!
3 participants