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

EVAL_HISTORY is busted #774

Closed
zenspider opened this issue Nov 20, 2023 · 7 comments · Fixed by #953
Closed

EVAL_HISTORY is busted #774

zenspider opened this issue Nov 20, 2023 · 7 comments · Fixed by #953
Labels
bug Something isn't working

Comments

@zenspider
Copy link

with IRB.conf[:EVAL_HISTORY] = true everything is busted:

9972 % irb
>> 1  + 2
/Users/ryan.davis/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb/ext/history.rb:127:in `>': comparison of Integer with true failed (ArgumentError)

      @contents.shift if @size != 0 && @contents.size > @size
                                                        ^^^^^
	from /Users/ryan.davis/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb/ext/history.rb:127:in `push'
	from /Users/ryan.davis/.rubies/ruby-3.2.2/lib/ruby/3.2.0/irb/ext/history.rb:26:in `set_last_value'

even irb_info

current .irbrc:

IRB.conf[:PROMPT_MODE]      = :SIMPLE
IRB.conf[:USE_AUTOCOMPLETE] = false
IRB.conf[:USE_COLORIZE]     = false

# maybe?

IRB.conf[:EVAL_HISTORY]     = true
@st0012 st0012 added the bug Something isn't working label Nov 20, 2023
@tompng
Copy link
Member

tompng commented Nov 21, 2023

IRB.conf[:EVAL_HISTORY] should be number or nil.

Do you think IRB needs type validation for configs?
Or EVAL_HISTORY=true should enable EVAL_HISTORY with some default size? (maybe about 10?)

These are some other confusing configs and invalid values that raise error.

IRB.conf[:IRB_NAME] = :ruby # should be string
IRB.conf[:IRB_RC] = '/path/to/.irbrc' # should be proc
IRB.conf[:BACK_TRACE_LIMIT] = ENV['IRB_BT_LIM'] # should be number
IRB.conf[:PROMPT] = '>> ' # should be hash

@st0012
Copy link
Member

st0012 commented Nov 22, 2023

Yeah both type validation and better documentation (discovery) are needed for those configs.

IRB.conf[:IRB_NAME] = :ruby # should be string

For this we should also support symbol, or even anything that responds to to_s?

Or EVAL_HISTORY=true should enable EVAL_HISTORY with some default size?

SAVE_HISTORY also doesn't support true (it has a default of 1000 though). So for consistency I'd say no, unless we want to change them both?

@zenspider
Copy link
Author

That's totally on me. I should have noticed that some default values were nil and some were not. I don't think we need full type checking, but some defensive programming would be nice... and doco. always doco

@zenspider
Copy link
Author

@st0012 want me to file a separate bug against doco and type checking?

@st0012
Copy link
Member

st0012 commented May 6, 2024

@zenspider Sorry for the delay. I've now cleared other priorities and will start looking at this. For the start I'll target the configs listed here, so feel free to add new ones.

@st0012
Copy link
Member

st0012 commented May 6, 2024

Actually, I think I'll close this with #953. If you find other configs can use the same improvement, please open new ones 😄

@zenspider
Copy link
Author

#953 feels like the right thing to do. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants