SCI_GETSEKTEX(0,0) returns incorrect result in 2.01?

5 views
Skip to first unread message

Greg Smith

unread,
Aug 20, 2009, 12:05:23 PM8/20/09
to scintilla...@googlegroups.com
In Editor.cxx, line 6468:

return selectedText.len + 1;

Is this correct? selectedText.len already is 1 longer, at least in the
stream selection case, to account for the '\0' on the end of the text.
Adding 1 here makes the returned result 1 too long... except, if you have no
selection, selectedText.len holds 0.

As we really have to use SCI_GETSELTEXT() now to find the length of the
selection, I would expect SCI_GETSELTEXT()-1 to be the number of selected
characters in the window. So would it be possible to either adjust
CopySelectionRange() to return "\0" and a .len of 1 for an empty selection,
or change line 6468 to:

return selectedText.len ? selectedText.len : 1;

A user of our code noticed this as our scripting language can report the
length of the selection. We used to work out the selection size using the
Caret position and the Anchor position, but this is no longer possible with
multiple selections, so we changed to using SCI_GETSELTEXT()-1, expecting
this to be the number of selected characters.

Greg Smith
Director


Cambridge Electronic Design Limited
Science Park, Milton Road, Cambridge CB4 0FE, UK
Web: http://www.ced.co.uk
Tel: +44 (0)1223 420186
Fax: +44 (0)1223 420488
Registered in the UK no. 972132


Neil Hodgson

unread,
Aug 20, 2009, 11:29:36 PM8/20/09
to scintilla...@googlegroups.com
Greg Smith:

> As we really have to use SCI_GETSELTEXT() now to find the length of the
> selection, I would expect SCI_GETSELTEXT()-1 to be the number of selected
> characters in the window. So would it be possible to either adjust
> CopySelectionRange() to return "\0" and a .len of 1 for an empty selection,
> or change line 6468 to:
>
>                 return selectedText.len ? selectedText.len : 1;

OK, committed.

Also changed the case where the allowLineCopy case of
CopySelectionRange was not counting the '\0'.

Neil

Greg

unread,
Aug 21, 2009, 5:18:59 AM8/21/09
to scintilla-interest
>    Also changed the case where the allowLineCopy case of
> CopySelectionRange was not counting the '\0'.
This is changing line 5333 in Editor::CopySelectionRange() to the
following?
ss->Set(textWithEndl, strlen(textWithEndl) + 1,

Neil Hodgson

unread,
Aug 21, 2009, 8:29:39 AM8/21/09
to scintilla...@googlegroups.com
Greg:

> This is changing line 5333 in Editor::CopySelectionRange() to the
> following?
>                        ss->Set(textWithEndl, strlen(textWithEndl) + 1,

Yes.

Neil

Greg

unread,
Sep 7, 2009, 9:57:34 AM9/7/09
to scintilla-interest
In the SCI_GETSELTEXT handler (with apologies for the title of this
thread) in Editor.cxx, a colleague has pointed out that the code looks
a bit odd for the case where the user wants to return the text
(lParam != 0):

char *ptr = CharPtrFromSPtr(lParam);
int iChar = 0;
if (selectedText.len) {
for (; iChar < selectedText.len; iChar++)
ptr[iChar] = selectedText.s[iChar];
} else {
ptr[0] = '\0';
}
return iChar;

If selectedText.len is 1 (implying an empty string), the return value
is 1, but there is a handler for selectedText.len being 0, which
generates a string containing '\0', but returns 0. If selectedText.len
can never be 0, then the code is redundant, otherwise the ptr[0] =
'\0'; case probably wants iChar = 1; adding. Alternately, a few words
are needed in the documentation.

Neil Hodgson

unread,
Sep 7, 2009, 7:12:53 PM9/7/09
to scintilla...@googlegroups.com
Greg:

> If selectedText.len is 1 (implying an empty string), the return value
> is 1, but there is a handler for selectedText.len being 0, which
> generates a string containing '\0', but returns 0. If selectedText.len
> can never be 0, then the code is redundant,

selectedText.len can be 0 since this is the value it is initialized to
and it retains this value when there is no need to allocate such as
when the selection is empty and allowLineCopy is false.

Neil

Reply all
Reply to author
Forward
0 new messages