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.

Sign up for GitHub

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

Jump to bottom

Implement remaining tests for $if directive #584

Closed
wants to merge 1 commit into from
Closed

Implement remaining tests for $if directive #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 104 additions & 52 deletions lib/reline/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'rubygems'

class Reline::Config
attr_reader :test_mode

Expand Down Expand Up @@ -45,6 +47,16 @@ class InvalidInputrc < RuntimeError
attr_accessor v
end

RUBY_OPERATOR_BY_INPUTRC_OPERATOR = {
'=': :==,
'==': :==,
'!=': :!=,
'<=': :<=,
'>=': :>=,
'<': :<,
'>': :>
}

attr_accessor :autocompletion

def initialize
Expand Down Expand Up @@ -227,18 +239,36 @@ def read_lines(lines, file = nil)
end

def handle_directive(directive, file, no)
directive, args = directive.split(' ')
directive, args = directive.split(/\s+/, 2)
args, _comment = args.split(/\s+#\s*/, 2) if args
Copy link
Member

Choose a reason for hiding this comment

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

Readline only supports /^#.*/ style comment and /^\$if\s+version[comparator][value]\s*#.*/ style comment.
It may look like supporting comment, but as far as readling gnu readline source code, it does not. $if mode = foo # comment is same as $if mode=foo garbage.

case directive
when 'if'
condition = false
case args
when 'mode'
when 'term'
when 'version'
else # application name
condition = true if args == 'Ruby'
condition = true if args == 'Reline'
end
lhs, operator, rhs = args.partition(/\s*(==|=|!=|<=|>=|<|>)\s*/)
stripped_operator = $1
Copy link
Member

Choose a reason for hiding this comment

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

I think stripped_operator is not needed. operator is enough.

ruby_operator = (RUBY_OPERATOR_BY_INPUTRC_OPERATOR[stripped_operator.to_sym] if stripped_operator)

condition =
if lhs == 'term' && operator == '='
rhs == ENV['TERM'] || rhs == ENV['TERM'].sub(/-.*\z/, '')
elsif lhs == 'mode' && operator == '='
editing_mode = %i[vi_insert vi_command].include?(@editing_mode_label) ? :vi : @editing_mode_label
rhs == editing_mode.to_s
elsif lhs == 'version' && %w[= == != >= <= < >].include?(stripped_operator)
# Comment syntax here is different from comment syntax above: `#` does
# not need to be preceded by whitespace
rhs_version_str, _comment = rhs.split(/\s*#\s*/)
rhs_version = Gem::Version.new(rhs_version_str)
reline_compatible_readline_version = Gem::Version.new(Reline::COMPATIBLE_READLINE_VERSION)
reline_compatible_readline_version.public_send(ruby_operator, rhs_version)
elsif %w[Ruby Reline].include?(args) # application name
true
elsif %w[= == !=].include?(stripped_operator) # variable
parse_variable(lhs, rhs).all? do |var_name, rhs_value|
Copy link
Member

Choose a reason for hiding this comment

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

parse_variable might return nil, it wrongly compares variables which readline does not.
I think validating lhs and then directly compare to instance variable is enough.

if VARIABLE_NAMES.include?(lhs) && get_value_as_string(lhs) == rhs.downcase
# readline 8.2

# on will set bell-style to audible
set bell-style on
# this is evaluated to true
$if bell-style = audible
# this is evaluated to false. "audible" != "on"
$if bell-style = on

# invalid value will set to off
set colored-completion-prefix on
set colored-completion-prefix offfff
# evaluated to true.
$if colored-completion-prefix = off
# evaluated to false because "off" != "offfff"
$if colored-completion-prefix = offfff

actual_value = instance_variable_get(:"@#{var_name}")
actual_value.public_send(ruby_operator, rhs_value)
end
end

@if_stack << [file, no, @skip_section]
@skip_section = !condition
when 'else'
Expand All @@ -256,81 +286,103 @@ def handle_directive(directive, file, no)
end
end

def bind_variable(name, value)
def parse_variable(name, value)
Copy link
Member

Choose a reason for hiding this comment

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

Can you revert this change? I think this change is not needed.
I added a comment where parse_variable is used.

case name
when 'history-size'
begin
@history_size = Integer(value)
{ history_size: Integer(value) }
rescue ArgumentError
@history_size = 500
{ history_size: 500 }
end
when 'bell-style'
@bell_style =
case value
when 'none', 'off'
:none
when 'audible', 'on'
:audible
when 'visible'
:visible
else
:audible
end
{
bell_style:
case value
when 'none', 'off'
:none
when 'audible', 'on'
:audible
when 'visible'
:visible
else
:audible
end
}
when 'comment-begin'
@comment_begin = value.dup
{ comment_begin: value.dup }
when 'completion-query-items'
@completion_query_items = value.to_i
{ completion_query_items: value.to_i }
when 'isearch-terminators'
@isearch_terminators = retrieve_string(value)
{ isearch_terminators: retrieve_string(value) }
when 'editing-mode'
case value
when 'emacs'
@editing_mode_label = :emacs
@keymap_label = :emacs
@keymap_prefix = []
{
editing_mode_label: :emacs,
keymap_label: :emacs,
keymap_prefix: []
}
when 'vi'
@editing_mode_label = :vi_insert
@keymap_label = :vi_insert
@keymap_prefix = []
{
editing_mode_label: :vi_insert,
keymap_label: :vi_insert,
keymap_prefix: []
}
end
when 'keymap'
case value
when 'emacs', 'emacs-standard'
@keymap_label = :emacs
@keymap_prefix = []
{
keymap_label: :emacs,
keymap_prefix: []
}
when 'emacs-ctlx'
@keymap_label = :emacs
@keymap_prefix = [?\C-x.ord]
{
keymap_label: :emacs,
keymap_prefix: [?\C-x.ord]
}
when 'emacs-meta'
@keymap_label = :emacs
@keymap_prefix = [?\e.ord]
{
keymap_label: :emacs,
keymap_prefix: [?\e.ord]
}
when 'vi', 'vi-move', 'vi-command'
@keymap_label = :vi_command
@keymap_prefix = []
{
keymap_label: :vi_command,
keymap_prefix: []
}
when 'vi-insert'
@keymap_label = :vi_insert
@keymap_prefix = []
{
keymap_label: :vi_insert,
keymap_prefix: []
}
end
when 'keyseq-timeout'
@keyseq_timeout = value.to_i
{ keyseq_timeout: value.to_i }
when 'show-mode-in-prompt'
case value
when 'off'
@show_mode_in_prompt = false
{ show_mode_in_prompt: false }
when 'on'
@show_mode_in_prompt = true
{ show_mode_in_prompt: true }
else
@show_mode_in_prompt = false
{ show_mode_in_prompt: false }
end
when 'vi-cmd-mode-string'
@vi_cmd_mode_string = retrieve_string(value)
{ vi_cmd_mode_string: retrieve_string(value) }
when 'vi-ins-mode-string'
@vi_ins_mode_string = retrieve_string(value)
{ vi_ins_mode_string: retrieve_string(value) }
when 'emacs-mode-string'
@emacs_mode_string = retrieve_string(value)
{ emacs_mode_string: retrieve_string(value) }
when *VARIABLE_NAMES then
variable_name = :"@#{name.tr(?-, ?_)}"
instance_variable_set(variable_name, value.nil? || value == '1' || value == 'on')
variable_name = :"#{name.tr(?-, ?_)}"
{ variable_name => value.nil? || value == '1' || value == 'on' }
end
end

def bind_variable(name, value)
parse_variable(name, value).each do |parsed_name, parsed_value|
instance_variable_set(:"@#{parsed_name}", parsed_value)
end
end

Expand Down
1 change: 1 addition & 0 deletions lib/reline/version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
module Reline
VERSION = '0.3.8'
COMPATIBLE_READLINE_VERSION = '8.2'
Copy link
Author

Choose a reason for hiding this comment

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

This is needed for the version test. I put the current version of Readline here, but I can't really say whether Reline is truly compatible with this version. Additionally, this will have to be kept up-to-date as necessary in order to properly support the version test.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the description.

whether Reline is truly compatible with this version

The first goal is to make $if version directive work, and I think we can't say we officially support 8.2. It's better to define this in config.rb as a private constant for now.

class Reline::Config
  ...
  COMPATIBLE_READLINE_VERSION = '8.0'
  private_constant : COMPATIBLE_READLINE_VERSION
end

For now, I think specifying major version 8.0 is better. (Until checking Reline really supports feature added between 8.0 and 8.2)

end
89 changes: 87 additions & 2 deletions test/reline/test_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,94 @@ def test_include
assert_equal :audible, @config.instance_variable_get(:@bell_style)
end

def test_if
def test_if_mode
@config.read_lines(<<~LINES.lines)
$if Ruby
set editing-mode vi
$if mode=vi # comment
set bell-style audible
$else
set bell-style visible
$endif
LINES

assert_equal :audible, @config.instance_variable_get(:@bell_style)
end

def test_if_term
original_term = ENV['TERM']
ENV['TERM'] = 'fake'

@config.read_lines(<<~LINES.lines)
$if term=fake # comment
set bell-style audible
$else
set bell-style visible
$endif
LINES

assert_equal :audible, @config.instance_variable_get(:@bell_style)
ensure
ENV['TERM'] = original_term
end

def test_if_term_prefix
original_term = ENV['TERM']
ENV['TERM'] = 'fake-color'

@config.read_lines(<<~LINES.lines)
$if term=fake # comment
set bell-style audible
$else
set bell-style visible
$endif
LINES

assert_equal :audible, @config.instance_variable_get(:@bell_style)
ensure
ENV['TERM'] = original_term
end

def test_if_version
@config.read_lines(<<~LINES.lines)
$if version >= 0.0#comment without whitespace # comment with whitespace
set bell-style audible
$else
set bell-style visible
$endif
LINES

assert_equal :audible, @config.instance_variable_get(:@bell_style)
end

def test_if_application
@config.read_lines(<<~LINES.lines)
$if Ruby # comment
set bell-style audible
$else
set bell-style visible
$endif
LINES

assert_equal :audible, @config.instance_variable_get(:@bell_style)
end

def test_if_variable_boolean
@config.read_lines(<<~LINES.lines)
set show-mode-in-prompt on
$if show-mode-in-prompt = on # comment
set bell-style audible
$else
set bell-style visible
$endif
LINES

assert_equal :audible, @config.instance_variable_get(:@bell_style)
end

def test_if_variable_string
@config.read_lines(<<~LINES.lines)
set editing-mode vi
$if editing-mode == vi # comment
set bell-style audible
$else
set bell-style visible
Expand Down