Menu

#1489 SCI_LINEDELETE works inconsistently with SCI_LINECOPY and SCI_LINECUT

Won't_Implement
open
nobody
scintilla (287)
2
2024-08-12
2023-07-27
Vic
No

The inconsistency is when you make a selection of several lines:
while sci_linecopy and sci_linecut are able to delete all the lines touched by that selection, the sci_linedelete still only deletes the line where the cursor ends up.

Many many users like the ability to quickly delete selected lines (without having to make sure they select from line-start or line-end). Just Googling " "SCI_LINEDELETE" selection " brings at least these pages:
https://community.notepad-plus-plus.org/topic/23096/improve-sci_linedelete-shortcut
https://superuser.com/questions/258345/how-do-i-make-notepad-delete-lines-like-eclipse-does
https://www.purebasic.fr/english/viewtopic.php?t=69686
https://bugzilla.xfce.org/show_bug.cgi?id=11168
https://github.com/notepad-plus-plus/notepad-plus-plus/issues/13921

and many more if you search ex "delete selected lines".

So a lot of duplication of effort across the board of editors that use same Scintilla backend.

If you accept this to be fixed/patched, then, IMHO, it would be best for sci_linedelete to behave similarly to the other 2 (cut, copy) in the edge cases like:
- starting selection from start of line and moving with arrow-up (or -down), when visibly, it appears that the initial line where cursor was, does not appear in selection, but still will need to be deleted .

Thanks for your attention!

Discussion

  • Neil Hodgson

    Neil Hodgson - 2023-07-30

    I'll accept a good complete patch that implements this and includes a test case in test/simpleTests.py.

     
    👍
    1
  • Vic

    Vic - 2023-08-25

    From what I have found, the changes would be made in Message::LineDelete: in this location:
    https://sourceforge.net/p/scintilla/code/ci/default/tree/src/Editor.cxx#l4017

    Please see the attached the compressed file of Editor.cxx. It's first time for me contributing code to Scintilla. I'm not familiar with Mercurial and how to make a patch... I could learn it next time if needed.

    Explanation of changes:
    Replaced the lines 4018-4020 of Message::LineDelete: with lines 4008-4011 Message::LineCut:, to make the range of chars to act on to be exactly the same as for LineCut and LineCopy.
    Also, I refactored a bit LineCopy to have exactly same start as LineCut, and now LineDelete, to make more obvious that all 3 prepare the enlarged range in identical ways.

    I have tested in Notepad++, by recompiling it after making the changes. (The executable can be downloaded here https://ci.appveyor.com/project/victorel-petrovich/notepad-plus-plus-vic/builds/47875550/artifacts , which needs be placed inside a folder of portable version from https://notepad-plus-plus.org/downloads/v8.5.6/)

    IMO the changes are very simple so would not need formal tests in python... (Which I also would not know how to make).

     

    Last edit: Vic 2023-08-25
    • Neil Hodgson

      Neil Hodgson - 2023-08-25

      I'm not familiar with Mercurial and how to make a patch...

      Make a patch in git as this will generally be compatible with other source code control systems.

      Also, I refactored a bit LineCopy

      Patches should be minimized and isolated as that makes them easier to review and to understand when examining history for maintenance activities such as fixing . Unnecessary changes should not be included.

      The refactor does not appear beneficial.

      IMO the changes are very simple so would not need formal tests in python... (Which I also would not know how to make).

      Tests are essential to prove that the feature works and that it continues to work when other changes are made. They ensure that the new behaviour is exactly what you want as you have checked the result. They help communicate the expected behaviour to maintainers and suggest additional areas to check.

      have tested in Notepad++, by recompiling it after making the changes

      Recompiling code does not check its behaviour.

       
      • Vic

        Vic - 2023-08-25

        I don't know how to make a patch in git either.
        On github.com I just know how to make code changes directly in the website (or copy them from a file in my computer), then "commit". There I also can compile online (appveyor.com).
        If you know a simple way to get the patch in github, please let me know: https://github.com/victorel-petrovich/notepad-plus-plus_vic/commit/0a289fbcdfed89ec82c41f0131145d21ac18c727

        I have forked your project here on sourcefourge, but I don't see how to make a patch.

        By the way, how about moving the project to github? :) easier for beginners to make contributions, as you can tell.

        OK about the insignificant refactoring.

        I agree in general with your positions on tests, but I'd make exception for simple and small code changes.
        And I couldn't find tests in simpleTests.py for LineCopy or LineCut that I could use as a starter/example of how to make for LineDelete.

        Recompiling code does not check its behaviour.

        No? :) I tested the behaviour, not just recompiled.

         

        Last edit: Vic 2023-08-25
  • Yaron

    Yaron - 2023-11-05
     
    • Neil Hodgson

      Neil Hodgson - 2023-11-06

      That's an opinion in the opposite direction: SCI_LINECUT is wrong and SCI_LINEDELETE is correct. That means that, to maintain current behaviour for those that want it, SCI_LINEDELETE should remain as is but a new command for deleting all lines containing the selection could be contributed with a new name. Perhaps LinesSelectedDelete / SCI_LINESSELEECTEDDELETE.

      And in the opposite direction, a CurrentLineCut / SCI_CURRENTLINECUT could be contributed by those that like that behaviour.

      Or we could leave it up to downstream projects to implement their own variations.

       
      • Yaron

        Yaron - 2023-11-06

        Thank you.

         
  • Neil Hodgson

    Neil Hodgson - 2024-08-12
    • Group: Initial --> Won't_Implement
     

Log in to post a comment.