Menu

#2363 SCI_PARAUPEXTEND

Bug
closed-fixed
nobody
5
2022-12-06
2022-11-14
pryrt
No

SCI_PARAUPEXTEND doesn't match expectations

If I have three "paragraphs": two which are "example" multiple times, enough that it triggers word-wrap, and the third being three newline-separated lines

If I start at the "3" on line #3 (the middle of the second paragraph) and use Ctrl+Shift+{ to trigger SCI_PARAUPEXTEND, instead of selecting to the beginning of the current paragraph, it selects to the beginning of the previous paragraph.

However, if I start at the "7" on line #7 (the middle of the third paragraph) and use Ctrl+Shift+{ to trigger SCI_PARAUPEXTEND, it selects to the start of the third paragraph as expected.

Using unshifted Ctrl+{ or Ctrl+} SCI_PARAUP/SCI_PARADOWN to navigate finds only four paragraphs, starting at 1, 3, 6, and 10... so obviously, it considers all of line 3 to be a single paragraph and the three lines 6-9 to be a single paragraph. So it seems strange that starting "in the middle of a paragraph" changes behavior for SCI_PARAUP/SCI_PARAUPEXTEND depending on whether that paragraph is a single line wrapped across multiple visible lines or a multiple lines without extra newlines between.

(Originally found in Notepad++ v8.4.7 which uses the v5.3.1 library, and verified in Windows SciTE v5.3.1, so it's definitely part of the scintilla library behavior)

Discussion

  • Neil Hodgson

    Neil Hodgson - 2022-11-14

    Paragraph movement was contributed long ago and I don't use it so I'll just try to explain what the code is doing.

    The ParaUp code is only concerned with document lines - wrapping has no effect. It only examines whether each document line is blank - contains nothing or just spaces and tabs.

    // In the code, lines are numbered from 0, not 1
    line--;
    while (line >= 0 && IsWhiteLine(line)) { // skip empty lines
        line--;
    }
    while (line >= 0 && !IsWhiteLine(line)) { // skip non-empty lines
        line--;
    }
    line++;
    

    From (3, which is 2 in code), it goes (line--) to 1, first loop skips 1 to 0, second loop skips 0 to -1, then final line++ -> 0 which is 1 to the user,

    From (7 (code: 6)), it goes (line--) to 5 which is not empty so the first loop doesn't run and the second loop goes to 4 then terminates as 4 is empty and the final line++ arrives at 5 which is line 6 for user.

    If you start from the 'e' immediately above the '7' then ParaUp goes to line 3.

     
  • pryrt

    pryrt - 2022-11-14

    My point is that ParaUp behaves differently depending on where in the paragraph you are in. If you are on the second (physical) line or beyond of a given paragraph, then ParaUp will take you to the start of that paragraph. If you are on the first (physical) line of a given paragraph, then ParaUp will take you to the start of the previous paragraph.

     
    • Neil Hodgson

      Neil Hodgson - 2022-11-15

      I think that is what the original developer intended as a way to move between paragraph start lines. ParaUp moves to the starting line of the current paragraph but, when at the starting line of a paragraph, move to the starting line of the previous paragraph.

      Its similar to word movement with Ctrl+Left: when inside a word, move to the start of that word but, when at word start, move to start of previous word.

       
      • pryrt

        pryrt - 2022-11-15

        Interestingly, I would say that your analogy works better for my argument. If you have the phrase the word, and the caret is immediately before the w, then the caret is at the beginning of the word word, and so a Ctrl+Left should take you to the; but if the caret is between the w and the o, then the caret is not at the beginning of the word word, so Ctrl+Left should take you to the start of word.

        Similarly, the digit 3 in my example paragraph is not at the beginning of the paragraph: it's well into the middle of that paragraph. The only place I would consider the caret to be at the beginning of the paragraph is when the caret is before the first e of that paragraph.

        But at this point, I can't do anything more to show you my perspective (or that of other users of software that uses the Scintilla library). It is now in your hands whether you consider this a bug, or not worth fixing because most users have just resigned themselves to the (seemingly) inconsistent behavior, or working as designed. It is your library, not mine, after all.

         
        • Neil Hodgson

          Neil Hodgson - 2022-11-15

          But at this point, I can't do anything more to show you my perspective (or that of other users

          The code is deliberate in its behaviour so it appears to me to be implementing the author's intention. Other contributors that have worked around here also appear satisfied by the behaviour. So I'm not going to treat it as a bug.

          So far, you have said what you don't like but you haven't defined the behaviour you do want. Alternative behaviour could be included as an option. Its also possible for applications to implement their own actions.

           
          • pryrt

            pryrt - 2022-11-16

            So far, ... you haven't defined the behavior you do want.

            In my example above, putting the caret at the 3and then doing ParaUp should take you to the line labled 3 (which is internally line 2). If the caret were already at the beginning of that line (before the e), then it would still take you to the beginning of the line labeled 1.

            This could be accomplished by changing

            // In the code, lines are numbered from 0, not 1
            line--;
            

            to

            // In the code, lines are numbered from 0, not 1
            if(current_caret_column==0) line--;
            

            (for a properly defined current_caret_column, which is presumably already available or easily obtainable)

             
            • Michael Heath

              Michael Heath - 2022-11-16

              I can duplicate the requested behaviour in WordPad by using Ctrl+Shift+Up. Shift+Up also gets the preceding EOL sequence. The caret disappears during the operation so cannot state where it actually is located.

              Notepad does a line based selection using Shift+Up and Ctrl+Shift+Up does nothing. Tested also the latest notepad with Windows 11 in VirtualBox.

              If changed, may break existing third party code that relies on it and disrupt existing users habits. I do not use these paragraph functions often, if ever, so I will not lose much sleep over a change. Perhaps could notify on scite-Interest and other related sites for any objections before changing?

              The motivation for the feature change would be just to be consistent with Wordpad. I do not know about other (word processor) applications and what they do in comparison.

              This is what I tested with:

              Sci::Position Document::ParaUp(Sci::Position pos) const {
                  Sci::Line line = SciLineFromPosition(pos);
                  const Sci::Position start = LineStart(line);
                  if (pos == start)
                      line--;
                  while (line >= 0 && IsWhiteLine(line)) { // skip empty lines
                      line--;
                  }
                  while (line >= 0 && !IsWhiteLine(line)) { // skip non-empty lines
                      line--;
                  }
                  line++;
                  return LineStart(line);
              }
              

              I tried with GetColumn(pos) though could not resolve the GCC errors about "discard the qualifier" so went with LineStart(line).

              It is more like Wordpad in selecting to the start of paragraph before proceding up to the next paragraph. If the paragraph is indented then it appears to operate similar to Wordpad.

               
  • Neil Hodgson

    Neil Hodgson - 2022-11-22

    Referred to the mailing lists for objections.

    I tried with GetColumn(pos) though could not resolve the GCC errors about "discard the qualifier" so went with LineStart(line).

    LineStart is more efficient but GetColumn could and should be made const.

     
  • Neil Hodgson

    Neil Hodgson - 2022-11-28
    • labels: --> scintilla, selection
    • status: open --> open-fixed
     
  • Neil Hodgson

    Neil Hodgson - 2022-11-28

    No objections on the mailing list so committed with [11b83e] .

     
    👍
    1

    Related

    Commit: [11b83e]

  • Neil Hodgson

    Neil Hodgson - 2022-12-06
    • status: open-fixed --> closed-fixed
     
  • Neil Hodgson

    Neil Hodgson - 2022-12-06

    No objections on the mailing list so committed with [11b83e] .

     

    Related

    Commit: [11b83e]


Log in to post a comment.

MongoDB Logo MongoDB