Inquirer.js: paginator breaks when using line breaks inside checkboxes

[dominikwilkowski]

Hey there,

I only tested this against checkboxes but when you add a line break into the name of a checkbox the paginator will miscalculate the lines. That results in a cut-off of the current position of the arrow and all sorts of ugliness :)
Same happens when you add a line break \n into the Separator btw.

[dominikwilkowski]

I get around this currently by using new Inquirer.Separator() for each extra line. But that's probably not the right thing to do ;)

[SBoudrias]

This example is broken for you? https://github.com/SBoudrias/Inquirer.js/blob/master/examples/long-list.js#L11

[dominikwilkowski]

Yes sir!
The problem here only occurs when you have a window that is smaller than the viewport we have set.
You can see the bug on larger terminals as well but it's not as visible. Notice the jump of the cursor. That is just amplified when you have less space and the cursor moves outside the viewport.

See gif below:

[dominikwilkowski]

Actually it doesn't matter how tall the console is. A line break will throw the calculation off and you notice that if you have more than one of them inside the code.

I just ran this:

/**
 * Paginated list
 */

'use strict';
var inquirer = require('inquirer');

var choices = [];

choices.push('Multiline option \n  super cool feature\n  super cool feature');
choices.push('Multiline option \n  super cool feature\n  super cool feature');
choices.push('Multiline option \n  super cool feature\n  super cool feature');
choices.push('Multiline option \n  super cool feature\n  super cool feature');
choices.push('Multiline option \n  super cool feature\n  super cool feature');
choices.push('Multiline option \n  super cool feature\n  super cool feature');

inquirer.prompt([
    {
        type: 'checkbox',
        name: 'name',
        message: 'Select the letter contained in your name:',
        paginated: true,
        choices: choices
    }
]).then(function (answers) {
    console.log(JSON.stringify(answers, null, '  '));
});

This results in this:

[souravray]

The culprit is this logic in utils/paginator.js

  var lines = output.split('\n');
 // Move the pointer only when the user go down and limit it to 3
  if (this.pointer < 3 && this.lastIndex < active && active - this.lastIndex < 9) {
    this.pointer = Math.min(3, this.pointer + active - this.lastIndex);
  }

This is an issue not just in case when you put hard line break in your choices text. If you have choices text too larger than cli width (default is 80 i guess), then this problem occurs during text wrapping.
Infact text wrapping cause much more comlicated issues like lionger than viewport text. becaus the text wrapping is called in the render method, and pagination is been calculated in before that.
For my custom prompt I handel these issue by using a custom implementation of paginator.js and using a pre-formator (also I need to do some indentation fixes in multiline lists) to format the choices text before calling pagination function.

[SBoudrias]

If you were able to fix the pagination, then a PR is welcomed.

[souravray]

@SBoudrias I am not sure about if it is the best way to solve the issue, that's why I was refrained from doing PR.
It is kinda brute force logic, I am doing the line calculating the position of the last visible line by simply calculating the line number till the last line of selected section (after wrapping the text) and passed that number to pagination function, if the number is not undefined then the pagination function bypass all pagination logic by doing

 var topIndex =  Math.max(0, lastVisibleLine - pageSize);

[NTag]

I quickly fixed the issue described by @dominikwilkowski for the case of a list. I am only talking about this issue and not the problem of the terminal size.

Actually, I think the Paginator works, but the active parameter received by the paginate method needs to be the current line numbers, and not just the selected choice's index. Of course they are the same when all your choices are single-line.

So I changed list.js so it computes the number of lines of the choices before the current one.

[joshkel]

Does #602 address this?

[tsapeta]

Hey @SBoudrias, any chance to get @NTag fix (https://github.com/NTag/Inquirer.js/commit/bddafcbdcfe16964f915ffe4bb53c6f80a1117fc) included here? I can open a PR for list and checkbox prompts if this fix looks good to you.

[SBoudrias]

Hey @tsapeta PR is welcomed. I'll try to review and merge asap.