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

Idea: option to behave like read -n nchars #18

Open
lorenzogrv opened this issue Jan 15, 2014 · 6 comments
Open

Idea: option to behave like read -n nchars #18

lorenzogrv opened this issue Jan 15, 2014 · 6 comments

Comments

@lorenzogrv
Copy link

The idea is to implement an option to allow reading n characters rather than waiting for a new line character before executing callback, just like read(1) has.

-n nchars
    read returns after reading nchars characters rather
    than waiting for a complete line of input.

@isaacs If you think this could be nice, I can dive by myself through the code and make a pull request.

Regards,
L.

@lorenzogrv
Copy link
Author

Oops! Just seen the following lines right now:

if (opts.num) {
    throw new Error('read() no longer accepts a char number limit')
}

I'm closing this issue because your thoughts seems obvious. Anyway i'm curious, was this feature removed because of a deficient implementation?

@isaacs
Copy link
Contributor

isaacs commented Jan 15, 2014

I'm perfectly happy re-opening this, and I'd accept a pull req to add the feature back, provided it doesn't break anything else.

It was removed because implementing it on top of the builtin readline module was tricky, and not using the builtin readline module is just painful. For example, is 12<backspace>3 4 characters or 2? what about ^W to delete a word, etc? How do you count UTF-8 sequences?

If you look at the builtin readline module in Node, you'll see that quite a lot of code has gone into making this work as humans expect it to, and there's a lot of unclear tradeoffs involved in getting it right for n-chars reading.

@lorenzogrv
Copy link
Author

Actually read behaves differently when the flag -n nchars is set. Assume always typing 12<backspace>3<enter> after the command:

$> read && echo $REPLY
13
13
$> read -n 3 && echo && echo $REPLY
12^?
12<backspace control character>��
$> read -n 4 && echo && echo $REPLY
12^?3
12<backspace control character>�3

It seems that read will not respond as an human will expect when the -n flag is active. It simply counts and stores each key stroke, and replies the full sequence.

Regarding the implementation on top of the builtin readline, the key to implement it is the undocumented keypress event that readline adds to its input stream. That is, rely on readline's line event when n is not present, rely on the input stream keypress event else case.

I have done it successfully while developing an interactive CLI based on "hit one key and it will do something" as design principle, resulting the following code. Please note this is a quick and dirty implementation, just an example of what I mean.

/**
 * reads n characters from stdin
 * if !n, it will read until enter is hit
 */
function readn( n, callback ){
  var rl = readline.createInterface({
    input: process.stdin,
    output: process.stdout
  });

  function done( data ){
    rl.close();
    callback( data );
  }

  if( !n ){
    return rl.on( 'line', done )
  }

  var count = 0, sequence = '';
  process.stdin.on( 'keypress', storeKey );

  function storeKey( ch, key ){
    sequence += ( key && key.sequence || ch );
    count++;
    if( count == n ){
      this.removeListener( 'keypress', storeKey );
      done( sequence );
    }
  }
}

@isaacs
Copy link
Contributor

isaacs commented Jan 15, 2014

Ok, well, if it doesn't break anything else this lib does, and the code isn't too complicated to manage, and it matches the behavior of read(1), then sure, patch welcome.

@isaacs isaacs reopened this Jan 15, 2014
@lorenzogrv
Copy link
Author

Nice!

I would like to start adding some tests before going in the wild. It would be my first time using tap so I will pull first those tests to ask some feedback from you.

My thoughts are taking care of how -n will interact with other options. I was wondering specially what means the following statement on the README file.

If silent is true, and the input is a TTY, then read will set raw mode, and read character by character.

It's simply an implementation note?

It seems logic to use num as the option name as it used to be. Do you agree?

@isaacs
Copy link
Contributor

isaacs commented Jan 15, 2014

Yeah, that sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants