-
Notifications
You must be signed in to change notification settings - Fork 87
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
allow space in config value #705
Conversation
test/reline/test_config.rb
Outdated
@@ -388,7 +388,8 @@ def test_additional_key_bindings_for_other_keymap | |||
"cd": "CD" | |||
set keymap emacs | |||
"ef": "EF" | |||
set editing-mode vi # keymap changes to be vi-insert | |||
# keymap changes to be vi-insert | |||
set editing-mode vi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although inline comment does not exist, we should support ignore-after-space feature because readline behave like inline comment exists and it is actually used.
# ignore after space
"\C-f": history-search-forward ignored-string
set editing-mode vi ignored-string
# do not ignore after space
set emacs-mode-string emacs mode string
I think it is better to leave the test as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it! Trimmed the comment before handling the line.
lib/reline/config.rb
Outdated
@@ -182,7 +182,7 @@ def read_lines(lines, file = nil) | |||
next if if_stack.any? { |_no, skip| skip } | |||
|
|||
case line | |||
when /^set +([^ ]+) +([^ ]+)/i | |||
when /^set +([^ ]+) +(.+)/i | |||
var, value = $1.downcase, $2 | |||
bind_variable(var, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For an example input set foo bar baz
, how about passing three values to bind_variable?
var = 'foo'
value = 'bar'
raw_value = 'bar baz'
bind_variable(var, value, raw_value)
Most variable still uses value, and some variables (that uses retrieve_string(value)
now) will use retrieve_string(raw_value)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Added raw_value
as a third argument to bind_variable()
This reverts commit 2438347.
lib/reline/config.rb
Outdated
@@ -173,7 +173,7 @@ def read_lines(lines, file = nil) | |||
|
|||
no += 1 | |||
|
|||
line = line.chomp.lstrip | |||
line = line.gsub(/#.*$/, '').chomp.strip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for making confusion, I think we don't need to explicitly implement inline comment because readline does not have it.
But we need to retain set editing-mode vi # pseudo inline comment
ignoring " # pseudo inline comment"
part because readline ignores it and act like inline comment exist in some directives.
It is already implemented in L186
# value ignores " # pseudo comment" part. raw_value does not.
var, value, raw_value = $1.downcase, $2.partition(' ').first, $2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because readline does not have it.
Sorry for not getting the whole picture 💦 Removed this change!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thank you 👍
(ruby/reline#705) * allow space in config value fix ruby/reline#657 * remove inline comments * Revert "remove inline comments" This reverts commit ruby/reline@2438347c1a10. * refactoring * remove unnecessary comment handling ruby/reline@d60f1e1e39
Fix #657 .
(By the way, please forgive me for using the master branch. I know that is bad practice🙇♂️)