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

Omit nesting_level, use indent_level to build prompt string #610

Merged
merged 1 commit into from
Jun 24, 2023

Conversation

tompng
Copy link
Member

@tompng tompng commented Jun 21, 2023

Description

The document comment says that the part %i of the prompt is indent_level.

irb/lib/irb.rb

Lines 182 to 192 in 1159c13

# Constants +PROMPT_I+, +PROMPT_S+ and +PROMPT_C+ specify the format. In the
# prompt specification, some special strings are available:
#
# %N # command name which is running
# %m # to_s of main object (self)
# %M # inspect of main object (self)
# %l # type of string(", ', /, ]), `]' is inner %w[...]
# %NNi # indent level. NN is digits and means as same as printf("%NNd").
# # It can be omitted
# %NNn # line number.
# %% # %

This pull request removes calculation of nesting_level introduced in #500 and #515 and use indent_level for building prompt string.

Concerns

In test_ruby_lex.rb, the word nesting_level is used instead of indent_level.

But if we want nesting_level in prompt string again, we can easily re-implement it like this

NESTING_LEVEL_COUNTING_TOKENS = %i[on_kw on_tlambda on_tlambeg on_lparen on_lbracket on_lbrace]
def calc_nesting_level(opens)
  opens.count do |t|
    NESTING_LEVEL_COUNTING_TOKENS.include?(t.event)
  end
end

@st0012
Copy link
Member

st0012 commented Jun 21, 2023

Looking at the commit that introduced Row, the assertion used RubyLex's @indent ivar but calling it assert_nesting_level.

Given that the same test also already had assert_indenting to check the output with indentation added, my guess is that it's just indent and nesting were somehow used interchangeably. And nesting_level was picked to avoid having both assert_indent and assert_indenting, which would be confusing.

@st0012 st0012 merged commit f01ff08 into ruby:master Jun 24, 2023
@tompng tompng deleted the prompt_nesting_level_to_indent_level branch June 25, 2023 04:26
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