Project

General

Profile

Feature #12659

Readline: expose rl_char_is_quoted_p setting

Added by georgebrock (George Brocklehurst) about 3 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:76750]

Description

I maintain an interactive Git shell 1 built in Ruby, which made extensive use of the Readline module. We found we outgrew the functionality provided by the module, so we built our own Gitsh::LineEditor module using Ruby's Readline module as a starting point and adding more GNU Readline features to it. I'd like to contribute the features I add back upstream to Ruby. This is the first of them.

This patch exposes rl_char_is_quoted_p 2 as Readline.quoting_detection_proc.

rl_char_is_quoted_p is used to specify a function that determines if a character is escaped. It's called with completer word break characters that would normally indicate the boundary between two arguments, so that Readline can determine how much text to pass to the completion proc. For example, if we were implementing a Unix shell in Ruby using Readline, and a user typed ls some\ fil<tab> by default we'd get "fil" passed to our completion proc, but what we'd really want would be "some\ fil". Setting rl_char_is_quoted_p allows us to implement this behaviour.

The C function we assign to rl_char_is_quoted_p is called with a char array and an index into it, we convert those into a Ruby string and character index and pass them on to the proc. The implementation accounts for multibyte characters.

There are tests included in the patch that demonstrate the feature, and here's a simple program that uses it:

require "readline"

completion_text = nil
Readline.completion_proc = -> (text) do
  completion_text = text
  ["completed"]
end
Readline.completer_word_break_characters = " "
Readline.completer_quote_characters = "\"'"
Readline.quoting_detection_proc = -> (text, index) do
  index > 0 && text[index-1] == "\\"
end

while line = Readline.readline("> ") do
  p line
  p completion_text
  completion_text = nil
end

This is my first contribution to Ruby, so I'm sure there'll be some changes I need to make before it's accepted. I hope to be able to follow up with a few more Readline features as I build more gitsh features.

Thanks to my colleague Adam Sharp for pairing with me on the multibyte string support.


Files

readline_quoting_detection_proc.patch (9.29 KB) readline_quoting_detection_proc.patch georgebrock (George Brocklehurst), 08/06/2016 12:55 AM

Associated revisions

Revision 402f0426
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

rl_char_is_quoted_p

  • ext/readline/readline.c (readline_s_set_quoting_detection_proc): support rl_char_is_quoted_p. [Feature #12659]
  • ext/readline/readline.c (readline_s_get_quoting_detection_proc): ditto.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56326 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 56326
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

rl_char_is_quoted_p

  • ext/readline/readline.c (readline_s_set_quoting_detection_proc): support rl_char_is_quoted_p. [Feature #12659]
  • ext/readline/readline.c (readline_s_get_quoting_detection_proc): ditto.

Revision 56326
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

rl_char_is_quoted_p

  • ext/readline/readline.c (readline_s_set_quoting_detection_proc): support rl_char_is_quoted_p. [Feature #12659]
  • ext/readline/readline.c (readline_s_get_quoting_detection_proc): ditto.

Revision 56326
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

rl_char_is_quoted_p

  • ext/readline/readline.c (readline_s_set_quoting_detection_proc): support rl_char_is_quoted_p. [Feature #12659]
  • ext/readline/readline.c (readline_s_get_quoting_detection_proc): ditto.

Revision 56326
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

rl_char_is_quoted_p

  • ext/readline/readline.c (readline_s_set_quoting_detection_proc): support rl_char_is_quoted_p. [Feature #12659]
  • ext/readline/readline.c (readline_s_get_quoting_detection_proc): ditto.

Revision 011e45c6
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

use rb_locale_str_new

  • ext/readline/readline.c (readline_char_is_quoted): use rb_locale_str_new with the length. [Feature #12659]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56330 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 56330
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

use rb_locale_str_new

  • ext/readline/readline.c (readline_char_is_quoted): use rb_locale_str_new with the length. [Feature #12659]

Revision 56330
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

use rb_locale_str_new

  • ext/readline/readline.c (readline_char_is_quoted): use rb_locale_str_new with the length. [Feature #12659]

Revision 56330
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

use rb_locale_str_new

  • ext/readline/readline.c (readline_char_is_quoted): use rb_locale_str_new with the length. [Feature #12659]

Revision 56330
Added by nobu (Nobuyoshi Nakada) almost 3 years ago

use rb_locale_str_new

  • ext/readline/readline.c (readline_char_is_quoted): use rb_locale_str_new with the length. [Feature #12659]

History

Updated by shevegen (Robert A. Heiler) about 3 years ago

If I understand it correctly then you provide a wrapper that is currently not part of ruby readline module yes?

If so then I am in full support of this.

Updated by georgebrock (George Brocklehurst) almost 3 years ago

Robert A. Heiler wrote:

If I understand it correctly then you provide a wrapper that is currently not part of ruby readline module yes?

If so then I am in full support of this.

Yes, that's exactly what this patch does.

Anything I can do to move this forward?

Updated by nobu (Nobuyoshi Nakada) almost 3 years ago

Could you add a few tests?

And these functions should be static, and don't seem need forward declarations if the latter function is defined before the former.

int readline_char_is_quoted(char *text, int index);
long byte_index_to_char_index(VALUE str, long byte_index);

Updated by nobu (Nobuyoshi Nakada) almost 3 years ago

Could you add a few tests?

Sorry, I missed the patch.

You may be able to use rb_str_sublen() instead of byte_index_to_char_index().

static int
readline_char_is_quoted(char *text, int byte_index)
{
    VALUE proc, result, str;
    long char_index;
    size_t len;

    proc = rb_attr_get(mReadline, quoting_detection_proc);
    if (NIL_P(proc)) {
        return 0;
    }

    len = strlen(text);
    if (byte_index < 0 || len < (size_t)byte_index) {
        rb_raise(rb_eIndexError, "invalid byte index (%d in %"PRIdSIZE")",
                 byte_index, len);
    }

    str = rb_locale_str_new_cstr(text);
    char_index = rb_str_sublen(str, byte_index);
    result = rb_funcall(proc, rb_intern("call"), 2, str, LONG2FIX(char_index));
    return RTEST(result);
}
#5

Updated by nobu (Nobuyoshi Nakada) almost 3 years ago

  • Status changed from Open to Closed

Applied in changeset r56326.


rl_char_is_quoted_p

  • ext/readline/readline.c (readline_s_set_quoting_detection_proc): support rl_char_is_quoted_p. [Feature #12659]
  • ext/readline/readline.c (readline_s_get_quoting_detection_proc): ditto.

Updated by georgebrock (George Brocklehurst) almost 3 years ago

Nobuyoshi Nakada wrote:

Applied in changeset r56326.

Thank you!

Also available in: Atom PDF