From d930a12e3782f0dc7fd91fffa61387c56d10e587 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 1 Jan 2021 21:46:58 +0000 Subject: [PATCH] additional testing for long line issue --- src/static/js/ace2_inner.js | 135 +++++++++++------------------ tests/frontend/specs/pageupdown.js | 36 +++++++- 2 files changed, 85 insertions(+), 86 deletions(-) diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index 5568e432b..ef80c8a1d 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -3068,6 +3068,7 @@ function Ace2Inner() { // 5, leaving us with [1,5] in array -- just makes sure we never // just a comfort blanket to make sure we never go too far // I'd like to see this above change removed before merging. + top.console.log('VIZZZY', scroll.getVisibleCharRange(rep)); const isPageDown = evt.which === 34; const isPageUp = evt.which === 33; @@ -3120,9 +3121,9 @@ function Ace2Inner() { // top.console.log(rep.selEnd[0] - (visibleLineRange[1] - visibleLineRange[0])); const targetLine = rep.selEnd[0] - (visibleLineRange[1] - visibleLineRange[0]); const beforeViewport = targetLine <= 0; - top.console.log('beforeVP', beforeViewport); - top.console.log('highlighting', highlighting); - top.console.log('shitKey', shiftKey); + // top.console.log('beforeVP', beforeViewport); + // top.console.log('highlighting', highlighting); + // top.console.log('shitKey', shiftKey); // if page up is above view port. if (beforeViewport || targetLine === 1) { @@ -3130,21 +3131,22 @@ function Ace2Inner() { rep.selEnd = [0, 0]; rep.selStart = [0, 0]; } - top.console.log('some shit...'); } else { // not above view port, so selection within visible document if (!shiftKey || highlighting) { rep.selStart[0] -= visibleLineRange[1] - visibleLineRange[0]; rep.selEnd[0] -= visibleLineRange[1] - visibleLineRange[0]; + let line; // need current character length of line try { line = rep.lines.atIndex(rep.selEnd[0]); } catch (e) { - // silently fail, no big deal.. + // silently fail, no big deal.. But this could be better line = rep.lines.atIndex(visibleLineRange[1]); } const lineLength = line.width; + // Keep the X character offset on page down if (previousCharacterOffset <= line.width) { rep.selStart[1] = previousCharacterOffset; @@ -3152,73 +3154,28 @@ function Ace2Inner() { } } } - /* - if (beforeViewport || targetLine === 1) { - // lets assume we select end of document then hit page up - if (highlighting) { - top.console.log('resetting...'); - // put the caret on the first line in the first position - if (shiftKey) rep.selStart = [0, 0]; - if (shiftKey) rep.selEnd = [0, 0]; - } else { - // we should always keep our selEnd.. - rep.selStart = [0, 0]; - if (!shiftKey) rep.selEnd = [0, 0]; - } - // TODO Handle long lines! - } else { - top.console.log('MAM'); - if (shiftKey) rep.selStart[0] -= visibleLineRange[1] - visibleLineRange[0]; - if (!shiftKey) rep.selEnd[0] -= visibleLineRange[1] - visibleLineRange[0]; - let line; - // need current character length of line - try { - line = rep.lines.atIndex(rep.selEnd[0]); - } catch (e) { - // silently fail, no big deal.. - line = rep.lines.atIndex(visibleLineRange[1]); - } - - const lineLength = line.width; - // Keep the X character offset on page down - if (previousCharacterOffset <= line.width) { - rep.selStart[1] = previousCharacterOffset; - if (!shiftKey) rep.selEnd[1] = previousCharacterOffset; - } - } - // TODO/JM: Handle page up in really long lines - */ } if (isPageDown) { - let line; - // need current character length of line - try { - line = rep.lines.atIndex(rep.selEnd[0]); - } catch (e) { - // silently fail, no big deal.. - line = rep.lines.atIndex(visibleLineRange[1]); - } - // Keep the X character offset on page down - if (previousCharacterOffset <= line.width) { - if (!shiftKey) rep.selStart[1] = previousCharacterOffset; - rep.selEnd[1] = previousCharacterOffset; - } - if (!shiftKey) rep.selStart[0] = rep.selStart[0] + (visibleLineRange[1] - visibleLineRange[0]); - rep.selEnd[0] = rep.selEnd[0] + (visibleLineRange[1] - visibleLineRange[0]); + // boolean - Can the editor see the next line? + const nextLineIsVisibleInViewport = rep.selEnd[0] <= visibleLineRange[1]; + top.console.log(rep.selEnd[0], visibleLineRange[1]); + top.console.log('nextLineIsVisibleInViewport', nextLineIsVisibleInViewport); - if (rep.selEnd[0] < 0 || rep.selStart[0] < 0) { - top.console.log('trying to visit a negative line?!'); - // don't go to negative lines.. - rep.selStart[0] = 0; - rep.selEnd[0] = 0; - return; + // lines that are very long might fill the entire page. + // if we can't see the end of the current line (IE see the next line) + // then we need to scroll the X position, not the Y + if (!nextLineIsVisibleInViewport) { + top.console.log('line too long, can nott see the next line'); + // TODO JM CAKE: Figure out how many chars are visible in viewport. + const visibleCharsInViewport = + getVisibleCharRangeOfLineInViewport(rep.lines.atIndex(rep.selEnd[0])); + top.console.log('erm', visibleCharsInViewport); + rep.selStart[1] += visibleCharsInViewport; + rep.selEnd[1] += visibleCharsInViewport; } - // if the new rep is beyond the viewport - // put the caret on the last line at the end of the line - if (rep.selStart[0] >= (linesLength - 1)) { - top.console.log('putting caret on the last line at the end of the line', rep.selStart[0], linesLength - 1); + if (nextLineIsVisibleInViewport) { let line; // need current character length of line try { @@ -3227,26 +3184,36 @@ function Ace2Inner() { // silently fail, no big deal.. line = rep.lines.atIndex(visibleLineRange[1]); } - let lineLength; - // if the last line is a blank line then don't throw an error. - if (line) { - lineLength = line.width; - } else { - lineLength = 0; + // Keep the X character offset on page down + if (previousCharacterOffset <= line.width) { + if (!shiftKey) rep.selStart[1] = previousCharacterOffset; + rep.selEnd[1] = previousCharacterOffset; } - if (!shiftKey) rep.selStart = [linesLength - 1, lineLength]; - rep.selEnd = [linesLength - 1, lineLength]; - } + if (!shiftKey) rep.selStart[0] = rep.selStart[0] + (visibleLineRange[1] - visibleLineRange[0]); + rep.selEnd[0] = rep.selEnd[0] + (visibleLineRange[1] - visibleLineRange[0]); - // lines that are very long might fill the entire page. - // if we can't see the end of the current line (IE see the next line) - // then we need to scroll the X position, not the Y - if (rep.selEnd[0] === visibleLineRange[1]) { - top.console.log('line too long, can\'t see the next line'); - // TODO JM CAKE: Figure out how many chars are visible in viewport. - const visibleCharsInViewport = getVisibleCharRangeOfLineInViewport(line); - if (!shiftKey) rep.selStart[1] += visibleCharsInViewport; - rep.selEnd[1] += visibleCharsInViewport; + // if the new rep is beyond the viewport + // put the caret on the last line at the end of the line + if (rep.selStart[0] >= (linesLength - 1)) { + top.console.log('putting caret on the last line at the end of the line', rep.selStart[0], linesLength - 1); + let line; + // need current character length of line + try { + line = rep.lines.atIndex(rep.selEnd[0]); + } catch (e) { + // silently fail, no big deal.. + line = rep.lines.atIndex(visibleLineRange[1]); + } + let lineLength; + // if the last line is a blank line then don't throw an error. + if (line) { + lineLength = line.width; + } else { + lineLength = 0; + } + if (!shiftKey) rep.selStart = [linesLength - 1, lineLength]; + rep.selEnd = [linesLength - 1, lineLength]; + } } } diff --git a/tests/frontend/specs/pageupdown.js b/tests/frontend/specs/pageupdown.js index a380e8b9c..77f74d2fa 100644 --- a/tests/frontend/specs/pageupdown.js +++ b/tests/frontend/specs/pageupdown.js @@ -213,7 +213,7 @@ describe('X character offset integrity is kept between page up/down', function ( }); }); -describe('Really long text line goes to character within text line if text line is last line in viewport', function () { +describe('Really long text line goes to character within text line if text line is last line in viewport if the second line is also incredibly long', function () { beforeEach(function (cb) { helper.newPad({ cb: async () => { @@ -247,7 +247,32 @@ describe('Really long text line goes to character within text line if text line 'hello world hello world hello world hello world hello world hello world hello world ' + 'hello world hello world hello world hello world hello world hello world hello world\n ' + 'hello world hello world hello world hello world hello world hello world hello world ' + - 'hello world hello world hello world hello world hello world hello world hello world '); + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world ' + + 'hello world hello world hello world hello world hello world hello world hello world\n\n\n\n\n\n\n\n\n '); cb(); }, }); @@ -269,6 +294,13 @@ describe('Really long text line goes to character within text line if text line return true; } }); + helper.pageUp(); + await helper.waitForPromise(() => { + if ((helper.padInner$.document.getSelection().anchorOffset > 0) && (helper.caretLineNumber() === 1)) { + throw new Error('This test will pass but it should not.. We need logic to check we were at the last possible caret accessible via the page up after the page down'); + return true; + } + }); }); });