Add support selection to EOL commands

[ArkadiuszMichalski]

Would be nice to have selection support for these two commands: EOL to Space and Remove Unnecessary Blank and EOL. Same as we have for trim operations #12655.

@donho I have a few questions before starting this. In EOL case it's a bit simpler, because here it's only important to select EOL, so what we select will be taken into account. So for EOL to Space:

Will replace EOL in 1 line.

Will replace EOL in 1 and 2 line.

Will replace EOL in 1 line.

In this case it doesn't make sense to extend the selection to the whole data? For example:

Expanding will not affect the process because the selected EOL will not change. Well, but I'm not sure to not expand anyway, because the Trim operations make expanding and this one EOL to Space should not do it? Also Remove Unnecessary Blank and EOL firstly run doTrim() so expanding will be done here anyway.
These two commands should be considered in the context of whole lines or just according to the exactly selection made? A similar question will appear later for the last 3 commands Tab to Space, Space to Tab (all/leading).

I have one more question about this command Remove Unnecessary Blank and EOL. Actually for such example:

we get this:

We get last 3 space because last 3 EOL was change to it. But this spaces are also not unnecessary, as command name suggest? Should not be removed?

One more example:

After all once space left. If we assume that for Remove Unnecessary Blank and EOL, actions for EOL always mean replacing them with a space, this might be correct.

[donho]

@ArkadiuszMichalski
It makes sense that trim operations make expanding because trim operate for leading and trailing - that means the whole line.
Expanding is not necessary for replacing EOL, because it's different from blank operations (as you said: Expanding will not affect the process because the selected EOL will not change). We don't need to align all operations to have exactly the same behaviour. The most important thing to users (IMO) is, each operation makes sense and does what we expected.

[donho]

@ArkadiuszMichalski

I have one more question about this command Remove Unnecessary Blank and EOL. Actually for such example:

Oups! I have to admit that "Remove Unnecessary Blank and EOL" is not well named. This command trim leading & tail then replace EOL with space:
https://community.notepad-plus-plus.org/topic/17609/remove-unnecessary-blank-and-eol

We get last 3 space because last 3 EOL was change to it. But this spaces are also not unnecessary, as command name suggest? Should not be removed?

Before changing its behaviour, I think it's worth to change the command name. What's your suggestion?

[alankilborn]

worth to change the command name. What's your suggestion?

It is difficult.
"Join Lines Minimally" is fairly accurate, but clearly overlaps with the standard "Join Lines".

[ArkadiuszMichalski]

@donho

We don't need to align all operations to have exactly the same behaviour. The most important thing to users (IMO) is, each operation makes sense and does what we expected.

OK, it's better to ask about it before making any changes.

Before changing its behaviour, I think it's worth to change the command name. What's your suggestion?

Trim both and EOL to Space or Trim all and EOL to Space. Now it's just a sequence 3 and 4 commands so mention them directly would be less confusing.

Edit: btw, EOL to Space is not strictly (it doesn't convert every EOL to spaces), it's more like joining lines, basically we use here _pEditView->execute(SCI_LINESJOIN) so nothing fancy.

We still have one space.

Join Lines command is very similar:

notepad-plus-plus/PowerEditor/src/NppCommands.cpp

Lines 1794 to 1805 in 3f0f6a2

	case IDM_EDIT_JOIN_LINES:
	{
	const pair<size_t, size_t> lineRange = _pEditView->getSelectionLinesRange();
	if (lineRange.first != lineRange.second)
	{
	auto anchorPos = _pEditView->execute(SCI_POSITIONFROMLINE, lineRange.first);
	auto caretPos = _pEditView->execute(SCI_GETLINEENDPOSITION, lineRange.second);
	_pEditView->execute(SCI_SETSELECTION, caretPos, anchorPos);
	_pEditView->execute(SCI_TARGETFROMSELECTION);
	_pEditView->execute(SCI_LINESJOIN);
	}
	}

but it doesn't operate on the entire document (without selection) and expand selection.

[ArkadiuszMichalski]

I made some initial changes for the tests #12711.

Is SCI_LINESJOIN not always working correct?

This looks like a problem on Scintilla's side.

[ArkadiuszMichalski]

For above case:
https://sourceforge.net/p/scintilla/bugs/2372/