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

Remove unused PROMPT_N #685

Merged
merged 1 commit into from
Aug 20, 2023
Merged

Remove unused PROMPT_N #685

merged 1 commit into from
Aug 20, 2023

Conversation

smmr0
Copy link
Contributor

@smmr0 smmr0 commented Aug 20, 2023

Since #500 (revision 364a6d56), the PROMPT_N prompt format is no longer used (explanation follows). This change removes it.


The only usage of PROMPT_N's value is in IRB::Irb#eval_input:

irb/lib/irb.rb

Lines 522 to 534 in 65e8e68

# Evaluates input for this session.
def eval_input
@scanner.set_prompt do
|ltype, indent, continue, line_no|
if ltype
f = @context.prompt_s
elsif continue
f = @context.prompt_c
elsif indent > 0
f = @context.prompt_n
else
f = @context.prompt_i
end

In order for @context.prompt_n to be used here, continue must be falsey and indent must be positive.

That block given to set_prompt is called only byRubyLex#prompt:

irb/lib/irb/ruby-lex.rb

Lines 157 to 161 in 65e8e68

def prompt(opens, continue, line_num_offset)
ltype = ltype_from_open_tokens(opens)
indent_level = calc_indent_level(opens)
@prompt&.call(ltype, indent_level, opens.any? || continue, @line_no + line_num_offset)
end

In order to satisfy the conditions above, opens must be empty (technically it could also be populated with only falsey values, but that's not the case here) and calc_indent_level(opens) must be positive.

RubyLex#calc_indent_level is defined as:

irb/lib/irb/ruby-lex.rb

Lines 365 to 388 in 65e8e68

def calc_indent_level(opens)
indent_level = 0
opens.each_with_index do |t, index|
case t.event
when :on_heredoc_beg
if opens[index + 1]&.event != :on_heredoc_beg
if t.tok.match?(/^<<[~-]/)
indent_level += 1
else
indent_level = 0
end
end
when :on_tstring_beg, :on_regexp_beg, :on_symbeg, :on_backtick
# No indent: "", //, :"", ``
# Indent: %(), %r(), %i(), %x()
indent_level += 1 if t.tok.start_with? '%'
when :on_embdoc_beg
indent_level = 0
else
indent_level += 1
end
end
indent_level
end

If opens is empty, then the block given to opens.each_with_index will never be evaluated, and calc_indent_level will return the initial value of indent_level: 0. Therefore calc_indent_level(opens) cannot be positive while opens is empty.

Since these conditions cannot be satisfied, PROMPT_N will never be used.


However, maybe it's unintended that PROMPT_N is unused now, in which case I can close this PR.

@st0012 st0012 requested a review from tompng August 20, 2023 00:18
@tompng
Copy link
Member

tompng commented Aug 20, 2023

Thanks for the investigation. The change looks good 👍

I've checked git log to find what PROMPT_N was doing before.
PROMPT_N and PROMPT_C is almost combined to PROMPT_C in IRB 1.1.0.
IRB 1.0.0

prompt_i> if 1
prompt_n> 2.
prompt_c> to_s
prompt_n> end
=> "2"

IRB 1.1.0, the first version of IRB integrated with Reline.

prompt_i> i█
↓
prompt_c> if 1
prompt_c> 2.
prompt_c> to_s
prompt_c> en█
↓
prompt_c> if 1
prompt_c> 2.
prompt_c> to_s
prompt_i> end
=> "2"

Since IRB 1.1.0 and before #500, PROMPT_N seems to be used only in a weird condition (indent > 0) and (continue || code_block_open) == false

result << @prompt.call(ltype, indent, continue || code_block_open, @line_no + line_num_offset)

code_block_open == false and indent > 0 conflicts. I think it was a bug that this condition is accidentally satisfied in some input before #500.

# example of prompt_n accidentally used
prompt_n> ((end

So I think PROMPT_N should be removed in IRB 1.1.0. Thank you for finding this 😄

@tompng tompng merged commit 66e69fa into ruby:master Aug 20, 2023
24 checks passed
st0012 added a commit that referenced this pull request Aug 22, 2023
They were removed in #685, but we should still keep them to avoid breaking
changes to tools like Chef.

https://github.com/chef/chef/blob/533ff089479763f29045e4e6ddf388b73fc99338/lib/chef/shell.rb#L138
tompng pushed a commit that referenced this pull request Aug 23, 2023
matzbot pushed a commit to ruby/ruby that referenced this pull request Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants