Skip to content
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.

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

Make multi-select Copy/Paste behaviour as same as column edit's one #14338

Conversation

donho
Copy link
Member

@donho donho commented Nov 8, 2023

Copy some multi-select texts and paste them make all text glued all together. This commit makes pasted texts separated by EOL, as column selection's Copy/Paste behaviour.

Ref: #14266 (comment)

Copy some multi-select texts and paste them make all text glued all together.
This commit makes pasted texts separated by EOL, as column selection's Copy/Paste behaviour.

Ref: notepad-plus-plus#14266 (comment)
@donho
Copy link
Member Author

donho commented Nov 8, 2023

@Ekopalypse @vinsworldcom
Could you check this PR please?

@vinsworldcom
Copy link

@Ekopalypse @vinsworldcom
Could you check this PR please?

I adds the newlines, but not quite as expected:

two = obj[""]
three = obj[""]
four = obj[""]

With the cursor at position 0 (to the left of the t in two):

  1. use ALT+SHIFT+Down to extend caret to the f in four.
  2. Next, use CTRL+SHIFT+Right to create a multi-selection of the words "two", "three" and "four".
  3. CTRL+C to copy the selected words
  4. Use the Right arrow key to move the multi-carets to between the double-quotes (on each line)
  5. CTRL+V

Outcome:

two = obj["two
three
four
"]
three = obj["two
three
four
"]
four = obj["two
three
four
"]

Expected (and what happens in BetterMultiSelection):

two = obj["two"]
three = obj["three"]
four = obj["four"]

Cheers.

@donho
Copy link
Member Author

donho commented Nov 8, 2023

@vinsworldcom
Thank you for testing.
After committing, I just realized there was a problem of copied text format, so I did a fix.
I just committed the fix.
Could you try the latest commit and confirm me if it's OK the current state of PR?

@vinsworldcom
Copy link

vinsworldcom commented Nov 8, 2023

Could you try the latest commit and confirm me if it's OK the current state of PR?

Better, but still not quite. Doing the same test above, the output it:

two = obj[""]
three = obj[""]
four = obj["two"]
            three
            four

Seems as though the paste starts from the "main" caret and then continues in a column down vs. inserting at each word at the respective multi-caret location it was cut from to the present location of that respective multi-caret.

Cheers.

@Yaron10
Copy link

Yaron10 commented Nov 8, 2023

Test1
Test2

Select Test1.
Press Ctrl and select Test2.
Copy.
Add a new line.
Paste.

Result:

Test1
Test2

Expected:

Test1Test2

@vinsworldcom
Copy link

vinsworldcom commented Nov 8, 2023 via email

@Yaron10
Copy link

Yaron10 commented Nov 9, 2023

@vinsworldcom,

Try it with

Test1 Test2

@donho
Copy link
Member Author

donho commented Nov 9, 2023

@Yaron10
Frankly, I've never liked the original copy/paste behaviour of multi-select - it makes no sense to me to put all words together without delimiter.
Using EOL might not be the preferred choice of all users, but at least users can distinguish the different selections/words they have copied. And since the multi-select paste feature needs such format, I think it's (EOL as delimiter) the best compromise.

@donho
Copy link
Member Author

donho commented Nov 9, 2023

@vinsworldcom
Thank you for your test.
The latest commit has fixed the problem you described.
I have tested with your example:

two = obj[""]
three = obj[""]
four = obj[""]

and it works as expected.

However, I use rarely the multi-edit with keyboard on Visual Studio. Could you check the latest commit and give me a feedback for the necessary enhancement to this PR?

@vinsworldcom
Copy link

@donho for the few quick tests I did, selecting from top-down, bottum-up, partial, whole-word, navigation with arrow-key and CTRL + arrow keys, HOME, END ... It seemed to me the same as BetterMultiSelection plugin. I had two instances open, one with this new commit and one with 8.5.8+BetterMultiSelection plugin - they both behaved the same for my limited testing.

I'm sure an immense amount of work dealing with corner cases - so thank you for your dedication to getting this done!

I'm going to remove BetterMultiSelection for the next few days while going through my normal editing and use of N++. I'll see if anything pops up.

Cheers.

@Yaron10
Copy link

Yaron10 commented Nov 9, 2023

@donho,

Frankly, I've never liked the original copy/paste behaviour of multi-select - it makes no sense to me to put all words together without delimiter.

The current master behavior can be useful in many cases.
But, I agree, in many other cases splitting the words would be preferred.


If a doc is read-only, the flag is removed after Ctrl+V.


Following @vinsworldcom STR in #14338 (comment).

Ctrl+V:

two "]
three "]
four "]

(Undo is disabled).

Edit -> Paste:

"]
"]
two "]
three 
four 

@Yaron10
Copy link

Yaron10 commented Nov 9, 2023

Use the Right arrow key to move the multi-carets to between the double-quotes (on each line)

I thought Shift should be pressed.

Edit -> Paste:

two = obj[""]
three = obj[""]
four = obj["two "]
            three 
            four 

@donho
Copy link
Member Author

donho commented Nov 10, 2023

@Yaron10
Thank you for testing and finding bugs.

If a doc is read-only, the flag is removed after Ctrl+V.

Fixed in the latest commit.

Edit -> Paste:

In the perfect world it should work also with Ctrl-V, but in real life ppl just don't use the the Paste on the menu item.
But we all wish be in the perfect world, so it's also fixed in the latest commit. :)

@Yaron10
Copy link

Yaron10 commented Nov 10, 2023

@donho,

I usually use Paste in the context menu.

Thank you for your time and effort.


Undo is disabled after Ctrl+V.


two = obj[""]
three = obj[""]
four = obj[""]
five = obj[""]

Use ALT+SHIFT+Down to extend caret to the f in four.
Next, use CTRL+SHIFT+Right to create a multi-selection of the words "two", "three" and "four".
CTRL+C to copy the selected words
--
Place the caret between "" in the last line.
Press Ctrl and place the caret between "" in the first line.
Press Ctrl and place the caret between "" in the second line.
Press Ctrl and place the caret between "" in the third line.
Paste.

Why should the result in the first three lines be different? - The main caret and additional carets are in the same positions as in @vinsworldcom's case.
IOW: I'm trying to understand if (numSelections == stringArray.size()).

@vinsworldcom
Copy link

@Yaron10 , if I do your latest test case in the latest commit artifact and in N++ with BetterMultiSelection, I get the same results - so at least behavior is consistent - if not ideal.

Cheers.

@donho
Copy link
Member Author

donho commented Nov 10, 2023

Use ALT+SHIFT+Down to extend caret to the f in four.
Next, use CTRL+SHIFT+Right to create a multi-selection of the words "two", "three" and "four".
CTRL+C to copy the selected words
--
Place the caret between "" in the last line.
Press Ctrl and place the caret between "" in the first line.
Press Ctrl and place the caret between "" in the second line.
Press Ctrl and place the caret between "" in the third line.
Paste.

Why should the result in the first three lines be different? - The main caret and additional carets are in the same positions as in

if I do your latest test case in the latest commit artifact and in N++ with BetterMultiSelection, I get the same results - so at least behavior is consistent - if not ideal.

In BetterMultiSelection & this PR current state, concerning behaviour is the same - it ensures both number of source (occurrences to paste in clipboard) and number of targets (multi-selection) are the same to do this operation.

@Yaron10 & @vinsworldcom Such behaviour can be changed - what's your preferred behaviour then?

@vinsworldcom
Copy link

The current behaviour is consistent with what I would expect - though I've been user BetterMultiSelection for years now so my view of behaviour is probably skewed to the way that plugin operates vs. what new possibilities could be.

I generally don't do the thing @Yaron10 posits above - manually clicking the paste-to locations in a different order expecting they would appear in that new order and perhaps repeat if more paste-to locations are selected than objects copied. I don't really have a guess as to how this should behave since I've never really had a use case for it.

With respect, I will caution that looking at these corner cases and creating an interpretation for them will surely be "incorrect" in at least someone else's opinion so not sure how far down the added implementation road you want to go?

Cheers.

@Yaron10
Copy link

Yaron10 commented Nov 10, 2023

As opposed to @vinsworldcom, I rarely use multiple-selections in real life.
If you think the current behavior is OK, so be it.

@Yaron10
Copy link

Yaron10 commented Nov 10, 2023

@donho,

Thank you for fixing the Undo issue.

@alankilborn
Copy link
Contributor

@donho said:

but in real life ppl just don't use the the Paste on the menu item.

Au contraire ... the only people that use the menu item for Paste are the ones that want to convert all line-endings in the current file. #9260 :-)

@donho donho closed this in 4ff9d77 Nov 10, 2023
@donho donho deleted the modify_multi-select_copy_behaviour_in_scintilla branch November 10, 2023 23:05
@alankilborn
Copy link
Contributor

@donho / @Yaron10

Let me say one more thing about this:

but in real life ppl just don't use the the Paste on the menu item.

@yaron noted:

I usually use Paste in the context menu.


Let's try to remember going forward that users have different ways of doing things. With configurable editor-context menu, configurable tab-right-click menu, mapped-keycombos, or just plain using something on the "main menu", ALL are equally valid ways to do something, and user picks which is best for himself. These things should all in the end execute the same code, no matter how invoked.

I'm not sure how "paste" ever became different depending upon how invoked...

I recently took care of the strangeness with Line Duplicate...

All of these are good steps.

@Ekopalypse
Copy link
Contributor

Sorry I'm late to the party, unfortunately I've been busy with other things all week.

I also tested the PR and ran into 3 "problems" (?).

  1. assuming two lines with the content
test1
test2

make a rectangular selection before 't'
jump to the end of the line and press Enter
-> works as expected

Now select a rectangular selection at the end of the line and press Enter. It should have the same behavior as the first test, but this is not the case. It just inserts a new line after test2 and also I can't use the backspace key anymore. If I move the cursor once to the left and then back to the right, before pressing enter, then it works as expected.

Assume the following content for the second test

test1
//
test2
//

Select both //, move the cursors up and to the first position, now press shift+arrow_down and shift+end
-> two selections, test1+crlf+//+crlf and test2+crlf+//+crlf should be selected.
Press ctlr+c and arrow_right and ctrl+v.

The result should be

test1
//test1
//
test2
//test2
//

but the result is

test1
//
test2
//test1
  //
  test2
  //

The second test is very specific, so it's not really a problem for me, but it might be worth thinking about.

Finally, assume the following code in python

def test1():
    pass

def test2():
    pass

Select both instances of pass and press enter
This will work if you have disabled the settings->preferences->auto-completion->auto-indent but NOT if this setting is active, which is the default.

@vinsworldcom
Copy link

Finally, assume the following code in python

def test1():
    pass

def test2():
    pass

Select both instances of pass and press enter This will work if you have disabled the settings->preferences->auto-completion->auto-indent but NOT if this setting is active, which is the default.

Yikes! Can't believe I missed that one. Perhaps since I was using the default settings in a fresh portable install to test. I always have that setting enabled and you're right, it doesn't work as expected, but does with BetterMultiSelection plugin enabled.

Cheers.

@Ekopalypse
Copy link
Contributor

Ekopalypse commented Nov 14, 2023

Just to make sure, all three tests behave differently and in my opinion correctly with the BetterMultiSelection plugin. The first and second are rather ignorable for me, the last one is more of a problem for me as I use npp 99% of the time with the autoindent feature.

@vinsworldcom
Copy link

the last one is more of a problem for me as I use npp 99% of the time with the autoindent feature.

Agree!

donho added a commit to donho/notepad-plus-plus that referenced this pull request Nov 14, 2023
Also disable auto-indent during multi-editing.

Ref: notepad-plus-plus#14338 (comment)
@donho
Copy link
Member Author

donho commented Nov 14, 2023

PR #14355 has enhanced multi-edit paste and Enter key-in.
It also disables auto-indent while multi-editing.

Please check it to see if it's what you need.

@vinsworldcom
Copy link

vinsworldcom commented Nov 14, 2023

It also disables auto-indent while multi-editing.

This is a non-starter for me. I always have auto-indent enabled and expect it to function during multi-caret editing - as it does today. Making it disabled during multi-caret editing breaks current capability and even with BetterMultiSelect, this will no longer work how it does today. I disabling auto-indent during multi-select as regressive.

NOTE: If this cannot be done "natively" - no worries. But please don't "break" functionality (disable auto-indent during multi-select editing) which leave me little way to get around that limitation. I'm happy to continue to use a plugin to get the functionality I expect, but disabling auto-indent during multi-select editing renders even the plugin route unusable.

Cheers.

donho added a commit that referenced this pull request Nov 15, 2023
Also disable auto-indent during multi-editing.

Ref: #14338 (comment)

Close #14355
@CennoxX
Copy link
Contributor

CennoxX commented Nov 15, 2023

If I use the builds of 1764758 then the deletions at the beginning/end of lines are without effect:

delete key on line end
If I press the delete key on the line end, while having multiple cursors, nothing happens.
I can simulate the expected behaviour by pressing [shift]+[right] and [delete]
output

backspace on line start
Same problem on line start, while having multiple cursors, and pressing backspace, though this works for example, if I pressed enter before.
output2

@vinsworldcom
Copy link

@CennoxX , indeed, I can confirm. With BetterMultiSelection, it does behave as expected - Delete at the end of the lines removes the "newline" and all the lines become one with multi-carets dispersed throughout continuing to delete as Del key is pressed.

Same with Backspace - although with BetterMultiSelection, no Enter is required first.

Cheers.

@donho
Copy link
Member Author

donho commented Nov 16, 2023

@CennoxX @vinsworldcom

If I press the delete key on the line end, while having multiple cursors, nothing happens.

I have no solution yet. I know it's not perfect, but in the meanwhile, use Ctrl-DEL for deleting the EOL in multi-edit mode.

donho added a commit to donho/notepad-plus-plus that referenced this pull request Nov 16, 2023
donho added a commit that referenced this pull request Nov 16, 2023
@donho
Copy link
Member Author

donho commented Nov 16, 2023

Please check new commit in master: c517985.
The code is different from #14359 after some refactoring. Though I have checked roughly, there could be some regressions for Multi-edition feature set.

Edit: the code was committed and pushed into master in a church, by hoping that the code is blessed and no more f**king bug found in multi-edit feature. :)

@vinsworldcom
Copy link

There seems to be some inconsistency in caret movement with multi carets. Use my previous example:


sub  {
    my ($) = @_;

    test
}


sub  {
    my ($) = @_;

    test
}

Highlight both "sub" and then press the down arrow key three times, to put the multi-carets on the lines with "test". They should both be at the same position in the line; however, they are not:

sub  {
    my ($) = @_;

|   test
}


sub  {
    my ($) = @_;

  |  test
}

Where the pipe (|) above shows where the carets end up for me. I would expect they should both be at the beginning of the "test" line, but the second one is 3 spaces in.

Cheers.

@donho
Copy link
Member Author

donho commented Nov 16, 2023

@vinsworldcom
Just checked with BetterMultiSelect, and found out that it cannot do such thing. Could you confirm that?
Otherwise, it seems such wrong behaviour is a bug of Scintilla. Could you create an issue in Scintilla project please?

@vinsworldcom
Copy link

vinsworldcom commented Nov 16, 2023

My bad, I should have said both carets should be at the 3-spaces in position, since sticky caret is enabled by default and the carets start at 3 positions in from the left on the "sub" line as they are placed after the b in "sub". The output expected should be:

sub  {
    my ($) = @_;

  |  test
}


sub  {
    my ($) = @_;

  |  test
}

Where the pipe (|) above shows where the carets end up for me when using BetterMultiSelection. This would be the case if using only a single caret - it would end up 3-spaces in due to sticky behavior. So with multi carets, they both should "obey" this rule.

I think the behavior of BetterMultiSelection is correct and both carets should end up 3 spaces in on the "test" line like I show in this post. My previous post was incorrect and for that I apologize.

Cheers.

@donho
Copy link
Member Author

donho commented Nov 16, 2023

@vinsworldcom

I think the behavior of BetterMultiSelection is correct and both carets should end up 3 spaces in on the "test" line like I show in this post. My previous post was incorrect and for that I apologize.

Strangely, I cannot make BetterMultiSelection work with my v8.5.8 (official release) - to select both sub with Ctrl + Mouse Duble Clicked, can you? Or you're using BetterMultiSelection with the binary of the current master?

@vinsworldcom
Copy link

I tried with both the latest artifact and the official 8.5.8 release - both with BetterMultiSelection - behave as described in my second post - with the carets being 3 spaces in on the "test" line after moving them down from the b on the "sub" line.

Note, you need to have multi-editing enabled:

image

It is not on by default in the 8.5.8 64-bit portable.

Cheers.

@donho
Copy link
Member Author

donho commented Nov 17, 2023

@vinsworldcom
Thank you for you explanation. I can reproduce your result now.

I tried with both the latest artifact and the official 8.5.8 release - both with BetterMultiSelection - behave as described in my second post - with the carets being 3 spaces in on the "test" line after moving them down from the b on the "sub" line.

Just tested in SciTE v5.3.9 - it's the same behaviour as under current master of Notepad++.
The bug has been reported here:
https://sourceforge.net/p/scintilla/bugs/2415/

Let's wait for the answer from Neil for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants