What would be the right behavior?

Mattias

During the revision of the StdCtrls documentation I came across an possible bug in TCustomComboBox.

Whenever the Sorted property is changed (SetSorted), UpdateSorted is called. It looks like UpdateSorted *always* sorts the list, as long as no handle (widget) has been created. This misbehaviour should affect at least the list items in design mode.

Can somebody confirm?

DoDi

procedure TCustomComboBox.UpdateSorted;
var
  lText: string;
  lIndex: integer;
begin
  if HandleAllocated then
    TWSCustomComboBoxClass(WidgetSetClass).Sort(Self, Items, FSorted)
  else if FItems is TStringList then
  begin
    // remember text
    lText := Text;
    TStringList(FItems).Sorted := FSorted;

When not FSorted then items are not sorted.

    lIndex := FItems.IndexOf(lText);
    if lIndex >= 0 then
      ItemIndex := lIndex;
  end;
end;

Mattias

When Sorted is False, the items should not be sorted.

DoDi

You're right.

But the following then is also evitable (but does no harm):

lIndex := FItems.IndexOf(lText); if lIndex >= 0 then ItemIndex := lIndex;

What if lIndex=-1?

end; end;

I stumbled across more such near-wrong code and comments, not worth bug reports (in customcombobox.inc):

line 55:

   if FSelStart  FSelLength then

Why not: if FSelLength > 0?

line 183:
{------------------------------------------------------------------------------ Method: TCustomComboBox.DoChange
Not followed by DoChange - should be moved, removed or corrected

line 675:

   // ToDo

Looks like Done (in InitializeWnd)

line 737:
{------------------------------------------------------------------------------

   function TCustomComboBox.GetDroppedDown: Boolean;

Should read "SetDroppedDown" - complete or remove

line 1064:
     Result := FReadOnly  false;
Should read: Result := FReadOnly;

Another (general) one:

I found sometimes wrong descriptions for ...Length of strings, mentioning *byte* count instead of count in *logical* UTF-8 chars. How should such descriptions finally read?

1) Should a logical character count be implied, so that only really different (byte/AnsiChar) counts should be mentioned explicitly?

2) Should count in [logical?] [UTF-8?] characters be mentioned wherever applicable? [what about possible future change to Unicode/UTF-16?]

DoDi

?

Hans-Peter Diettrich hat am 23. September 2011 um 05:40 geschrieben:

Nothing.?
?
?

Why not? if (FSelStart>0) or (fSelLength>0)??
?
?

fixed ?
?
?

line 675: ? ? // ToDo Looks like Done (in InitializeWnd)?

removed?
?
?

removed?
?

line 1064: ? ? ? Result := FReadOnly false; Should read: Result := FReadOnly;?

IMO such things are the programmers choice.
If we would start changing such things we would have to reject many patches. ? ?
?

I think it should be mentioned wherever applicable. Of course it is enough to add a link to a place where it is explained in detail. ? ??
Mattias
?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20110923/8797f859/attachment.html>

Obviously - but why?

Thanks for the fixes :-)

I'm using the LCL since many years now, and I never noticed whether the LCL components use Ansi or UTF-8 strings. What's the default, and where is it (or should be) described?

DoDi

What else do you expect to happen?

[...] I'm using the LCL since many years now, and I never noticed whether the LCL components use Ansi or UTF-8 strings. What's the default, and where is it (or should be) described?

I guess with 'Ansi' you mean system encoding. Where does the LCL use system encoding?

Mattias

ItemIndex -1 means the text does not exist in the list.

Mattias

I wonder e.g. why ItemIndex is not tested before all further processing. When nothing has been selected before sorting, then after sorting nothing should be/become selected as well.

I'm just asking. I think that I've seen such behaviour somewhere, will watch for concrete occurences.

DoDi