Menu

#1663 SCI_LINESJOIN and SCI_LINESSPLIT exceed selection boundary

Bug
closed-invalid
scintilla (607)
5
2014-10-21
2014-10-12
No

If a selection ends on an EOL, and is converted to target, followed by execution of SCI_LINESJOIN or SCI_LINESSPLIT, the line following the selection is included in the lines that are joined or split.

Attached screengrabs from Notepad++ exhibit the problem with Join. My expectation is that the "ff" line is not included in the join.
Notepad++'s code for "join" simply applies SCI_TARGETFROMSELECTION, then SCI_LINESJOIN, and similarly for "split".

2 Attachments

Discussion

  • Neil Hodgson

    Neil Hodgson - 2014-10-12

    The caret is at the start of the 'ff' line so the selection extends over the EOL.

     
  • Mike Cowperthwaite

    I maintain that is inconsistent, confusing, and incorrect.

    For comparison, SCI_TAB and SCI_BACKTAB do not affect the indentation of the line where the cursor sits at column 0, in a multi-line select.

     

    Last edit: Mike Cowperthwaite 2014-10-12
    • Neil Hodgson

      Neil Hodgson - 2014-10-12

      If you want a different range to be joined then ask for that by setting the target to the range you want before calling SCI_LINESJOIN.

       
  • Stan S

    Stan S - 2014-10-12

    By observation, SCI_LINESJOIN is very simple that it only replaces any newlines with a space.

    By testing, the newline of a row is only selected by physically placing the cursor on the nextline.

    Mike seems to be suggesting a sort of visually logical line join that for example when 3 lines are selected, that then is indicating those are the lines to be joined, and one line is the "result line".

    Perhaps his argument is that if more than one line is being selected, each line of course having a newline at its end, then at least one newline should be left or put at the end of joining multiple lines.

    Programmatically it seems that would mean that least the first character of a line should be highlighted to be then included in a joined line. A sort of disregard to where the cursor is, but on what text is highlight, i.e., the cursor must be at column 1 to include the line in the "result line".

    My thought is how will such a change affect other plugins if at all? Would creating a visually smarter line join in a text plugin of its own or for example a Python Script plugin to suffice as a workaround? And if not, why is SCI_LINESJOIN just so simple.

    Again, as it stands now, SCI_LINESJOIN replaces newlines with spaces, that's it.

    The suggestion seems to be should SCI_LINESJOIN ignore the last newline that is highlighted or add a newline, to create a one row "result line", given that more than one line is highlight, i.e., at least one row and at least one character of the next line, if the cursor is at column 0 of a newline.

    Interesting.

     
  • Neil Hodgson

    Neil Hodgson - 2014-10-13

    I can't see a bug here so setting status to invalid.

     
  • Neil Hodgson

    Neil Hodgson - 2014-10-13
    • labels: --> scintilla
    • status: open --> open-invalid
    • assigned_to: Neil Hodgson
     
  • Mike Cowperthwaite

    You have the right of fiat. That doesn't make you correct.

     
    • Neil Hodgson

      Neil Hodgson - 2014-10-13

      To me, you appear to be complaining about the way that NotePad++ is using Scintilla. How is this a bug in Scintilla?

       
  • Mike Cowperthwaite

    Its really hard for me to understand just what you are not understanding so, in these situations, I often just repeat what I said before.
     — Neil Hodgson, 18-Jul-2014

    Unfortunately, I lack that discipline, and furthermore get myself into a dudgeon.

    As simply as I can: The operations in question affect a line beyond the selection (which, for this discussion, can be thought of as equivalent to the target). SCI_TAB behaves in the manner I expect, and does not affect the line after the selection, so it seems clear that changing the text after the selection is not a policy.

    While Join might, conceivably, be defended as "removing all newlines within the selection," Split's behavior has no such fallback position.

    The behavior I seek would be achieved by adding these lines:

    if (pdoc->GetColumn(targetEnd) == 0)
        targetEnd -= pdoc->eolMode ? 1 : 2;  // or is there a better idiom for this?
    

    to both LinesJoin() and LinesSplit(), at the beginning of the "if" clause of each function.

    It is true, I can do almost the same thing in Notepad++ prior to calling these Scintilla functions, except I have to change the selection, then set the target, and then change the selection back to what the user had set. And so does every other Scintilla-based app that wants the same sensible behavior I'm seeking.

     
  • Neil Hodgson

    Neil Hodgson - 2014-10-13

    Think of SCI_LINESJOIN as a function that takes a range argument. That argument is passed through the target. By avoiding interpretation of that argument, a wide set of behaviours can be implemented by the application, including operations that do not involve the selection.

    "... except I have to change the selection, then set the target, and then change the selection back to what the user had set"

    You don't have to go through all these steps. Why call SCI_TARGETFROMSELECTION when you want a range that is a modified form of the selection? Why not set the target directly to what you want without changing the selection?

     
  • Mike Cowperthwaite

    OK, I missed the set-target API. I drop that one objection.

    I continue to disagree on the correct "interpretation" of the range. I am annoyed that you continue to ignore the inconsistency with SCI_TAB's behavior. But I have fulminated enough.

     
    • Neil Hodgson

      Neil Hodgson - 2014-10-13

      I do not see any inconsistency since SCI_LINESJOIN does not operate on the selection. It does one simple job accurately. It is up to the application author to build their preferred commands from that. Making SCI_LINESJOIN work in your preferred way may break applications that depend on it doing exactly what they ask it to.

       
  • Neil Hodgson

    Neil Hodgson - 2014-10-21
    • status: open-invalid --> closed-invalid
     

Log in to post a comment.