Skip to content

fix: resolve incorrect constraints for scrollable height in the REPL's pager #2293

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 2, 2024
Merged

fix: resolve incorrect constraints for scrollable height in the REPL's pager #2293

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/node_modules/@stdlib/repl/lib/output_stream.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
var out;
var i;

lines = str.split( RE_EOL ); // FIXME: this approach is not robust for three reasons: (1) line breaks may include carriage returns, not just line feeds (so the `+1` may not be appropriate), (2) it does not consider line feeds which are escaped (e.g., `\\\n`), and (3) assumes that line breaks map directly to displayed lines (while true most of the line for text which has been wrapped to 80 chars, such as repl.txt files, this may not be true for all outputs, some of which may spill across multiple lines)

Check warning on line 71 in lib/node_modules/@stdlib/repl/lib/output_stream.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unexpected 'fixme' comment: 'FIXME: this approach is not robust for...'
out = [ 0 ];
for ( i = 0; i < lines.length-1; i++ ) {
out.push( out[ i ] + lines[ i ].length + 1 );
Expand Down Expand Up @@ -271,7 +271,7 @@
if ( vh < MIN_VIEWPORT_HEIGHT ) {
return -1;
}
return vh - RESERVED_PAGING_ROWS;
return vh - RESERVED_PAGING_ROWS; // FIXME: reserved paging rows being a constant assumes that the input prompt spans just 1 row. This might not be the case with multi-line inputs triggering the pager as it will result in an incomplete UI with only the last line of the multi-line input visible in the pager view.

Check warning on line 274 in lib/node_modules/@stdlib/repl/lib/output_stream.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unexpected 'fixme' comment: 'FIXME: reserved paging rows being a...'
});

/**
Expand All @@ -285,8 +285,8 @@
* @returns {boolean} boolean indicating whether content is "scrollable"
*/
setNonEnumerableReadOnly( OutputStream.prototype, '_isScrollable', function isScrollable( N ) {
var h = this._pagerHeight();
return ( h > 0 && N > h );
var vh = this._repl.viewportHeight();
return ( vh >= MIN_VIEWPORT_HEIGHT && N > vh - 2 ); // 2 rows reserved for the input and the next prompt
});

/**
Expand Down
Loading