allow space in config value #705
Conversation
test/reline/test_config.rb
Outdated
| "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.
There was a problem hiding this comment.
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 stringI think it is better to leave the test as is.
There was a problem hiding this comment.
Got it! Trimmed the comment before handling the line.
lib/reline/config.rb
Outdated
| when /^set +([^ ]+) +([^ ]+)/i | ||
| when /^set +([^ ]+) +(.+)/i | ||
| var, value = $1.downcase, $2 | ||
| bind_variable(var, value) |
There was a problem hiding this comment.
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.
Thanks! Added raw_value as a third argument to bind_variable()
This reverts commit 2438347.
lib/reline/config.rb
Outdated
| no += 1 | ||
|
|
||
| line = line.chomp.lstrip | ||
| line = line.gsub(/#.*$/, '').chomp.strip |
There was a problem hiding this comment.
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, $2There was a problem hiding this comment.
because readline does not have it.
Sorry for not getting the whole picture 💦 Removed this change!
(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
3 participants
Fix #657 .
(By the way, please forgive me for using the master branch. I know that is bad practice🙇♂️)