Patch for fetching sel.MoveExtends()

[Mitchell]

Hi,

Attached is a simple patch for `SCI_GETSELECTIONMODE` that returns a new constant `SC_SEL_STREAMMOVEEXTENDS` if `sel.MoveExtends()` is active. This constant is read-only, and is ignored if passed to `SCI_SETSELECTIONMODE`.

When an application calls `SCI_SETSELECTIONMODE(SC_SEL_STREAM)`, subsequent movements like `SCI_CHARRIGHT` and `SCI_CHARLEFT` extend the selection without the need to call `SCI_CHARRIGHTEXTEND` and `SCI_CHARLEFTEXTEND`. For applications that want to perform more complex movements (such as move or select to the next instance of a particular character on the current line), the application needs to know what "mode" it is in and update the selection accordingly. Forcing the application to keep track of its own state flag is not reasonable since many Scintilla edit actions will change `sel.MoveExtends()` without notifying the container.

This patch is against Scintilla 3.7.x since I am not currently able to compile Scintilla 4.x due to hardware limitations.

Cheers,
Mitchell

[Neil Hodgson]

Mitchell:

> Attached is a simple patch for `SCI_GETSELECTIONMODE` that returns a new constant `SC_SEL_STREAMMOVEEXTENDS` if `sel.MoveExtends()` is active. This constant is read-only, and is ignored if passed to `SCI_SETSELECTIONMODE`.

Needs to be documented in ScintillaDoc.html, particularly as it has no effect in SCI_SETSELECTIONMODE.

Neil

[Mitchell]

Okay, please see the new patch, which is against 4.0.2. After some further thought, I tweaked the behavior such that `SCI_GETSELECTIONMODE` will return `SCI_SEL_STREAMDEFAULT` or `SCI_SEL_RECTANGLEDEFAULT` if `SCI_SETSELECTIONMODE` was not previously set. Otherwise, when `sel.MoveExtends()` is active, `SCI_GETSELECTIONMODE` will return whatever was passed to `SCI_SETSELECTIONMODE`. I think this behavior makes more sense, and is documented in the patch.

Cheers,
Mitchell

[Neil Hodgson]

Mitchell:

> Okay, please see the new patch, which is against 4.0.2. After some further thought, I tweaked the behavior such that `SCI_GETSELECTIONMODE` will return `SCI_SEL_STREAMDEFAULT` or `SCI_SEL_RECTANGLEDEFAULT` if `SCI_SETSELECTIONMODE` was not previously set. Otherwise, when `sel.MoveExtends()` is active, `SCI_GETSELECTIONMODE` will return whatever was passed to `SCI_SETSELECTIONMODE`. I think this behavior makes more sense, and is documented in the patch.

This appears to be less compatible with current applications. Performing an internet search for SCI_GETSELECTIONMODE shows code like:

if ((*_ppEditView)->execute(SCI_GETSELECTIONMODE) == SC_SEL_RECTANGLE)

The above code will not work when the user performs a rectangular selection by holding down the Alt key. Using move-selects is less common than Alt key rectangular selection.

Adding new return values is an incompatibility but I thought it would be a minor issue with the previous proposal. Perhaps this would be better as an additional API SCI_GETMOVESELECTS?

Extra benefit if there can be a SCI_SETMOVESELECTS so an app can save and restore the state more accurately. Setting would have to be performed in the order: SELECTIONMODE, MOVESELECTS as SCI_SETMOVESELECTS changes the move-selects state.

Neil

[Mitchell]

Hi Neil,

On Sun, 12 Nov 2017, Neil Hodgson wrote:

> Mitchell:
>
>> Okay, please see the new patch, which is against 4.0.2. After some further thought, I tweaked the behavior such that `SCI_GETSELECTIONMODE` will return `SCI_SEL_STREAMDEFAULT` or `SCI_SEL_RECTANGLEDEFAULT` if `SCI_SETSELECTIONMODE` was not previously set. Otherwise, when `sel.MoveExtends()` is active, `SCI_GETSELECTIONMODE` will return whatever was passed to `SCI_SETSELECTIONMODE`. I think this behavior makes more sense, and is documented in the patch.
>
> This appears to be less compatible with current applications. Performing an internet search for SCI_GETSELECTIONMODE shows code like:
>
> if ((*_ppEditView)->execute(SCI_GETSELECTIONMODE) == SC_SEL_RECTANGLE)
>
> The above code will not work when the user performs a rectangular selection by holding down the Alt key. Using move-selects is less common than Alt key rectangular selection.

Yes, but they should be using `SCI_SELECTIONISRECTANGLE` instead.

> Adding new return values is an incompatibility but I thought it would be a minor issue with the previous proposal.

Yes, but I figured since 4.x is a pretty big break from 3.x, why not make the change now? Also, it seems to me that `SCI_SETSELECTIONMODE` and `SCI_GETSELECTIONMODE` were designed more for `sel.MoveExtends()` than for generic selection state queries. Or was `sel.MoveExtends()` added as an after effect?

> Perhaps this would be better as an additional API SCI_GETMOVESELECTS?
>
> Extra benefit if there can be a SCI_SETMOVESELECTS so an app can save and restore the state more accurately. Setting would have to be performed in the order: SELECTIONMODE, MOVESELECTS as SCI_SETMOVESELECTS changes the move-selects state.

I don't think either is necessary based on my rationale above.

Cheers,
Mitchell

[Lex Trotman]

The documentation needs start indicating these areas that 4.x differs
from 3.x to assist when applications came to upgrade.

Cheers
Lex

[Neil Hodgson]

Mitchell:

>> Adding new return values is an incompatibility but I thought it would be a minor issue with the previous proposal.
>
> Yes, but I figured since 4.x is a pretty big break from 3.x, why not make the change now?

Incompatible changes have to be weighed against the cost of breakage and upset users, balancing the degree of improvement. The improvement doesn’t seem to clarify the API that much - to me making ‘moveExtends’ more independent in the API is clearer.

> Also, it seems to me that `SCI_SETSELECTIONMODE` and `SCI_GETSELECTIONMODE` were designed more for `sel.MoveExtends()` than for generic selection state queries. Or was `sel.MoveExtends()` added as an after effect?

These features were from Philippe Lhoste in 2003 and the main thrust was to allow modal rectangular selection by keyboard.
https://sourceforge.net/p/scintilla/code/ci/ed444ac1a5fcda07beb1787a088436eb29df1d66/

> Yes, but they should be using `SCI_SELECTIONISRECTANGLE` instead.

One of the comments from the patch thread (attached) is that SCI_GETSELECTIONMODE makes SCI_SELECTIONISRECTANGLE obsolete.

Neil

[Mitchell]

Hi Neil,

On Mon, 13 Nov 2017, Neil Hodgson wrote:

> Mitchell:
>
>>> Adding new return values is an incompatibility but I thought it would be a minor issue with the previous proposal.
>>
>> Yes, but I figured since 4.x is a pretty big break from 3.x, why not make the change now?
>
> Incompatible changes have to be weighed against the cost of breakage and upset users, balancing the degree of improvement. The improvement doesn’t seem to clarify the API that much - to me making ‘moveExtends’ more independent in the API is clearer.

Okay, please see the attached patch that adds a new `SCI_GETMOVEEXTENDSSELECTION()` message and documentation.

Cheers,
Mitchell

[Neil Hodgson]

Mitchell:

> Okay, please see the attached patch that adds a new `SCI_GETMOVEEXTENDSSELECTION()` message and documentation.

Righteo, committed:
https://sourceforge.net/p/scintilla/code/ci/85205da6ec1b8eeb14c3edc94064fb0d8f685377/

Neil