ButtonEdit Component

[JuhaManninen]

I tried to checkout your repo using EasyMercurial, but it failed.
Luckily I could use svn to check it out.

Bart, GitHub uses Git, not Mercurial. I don't know how you managed to use SVN with a Git repo.

I propose some changes.

Do they apply with the latest version from derit?
For some reason he continues developing this separate package instead of the lazarus branch which I requested.
I think we move the development to Lazarus trunk in any case. If if breaks temporarily, I don't care. It is time to get this component fixed.

[Leledumbo]

For some reason he continues developing this separate package instead of the lazarus branch which I requested.

Would you please guide him? There might be little communication problem due to his english, but that's just a small problem. The bigger one lies in his experience in version control system (especially writing a patch) and understanding LCL. Seriously I need T*Edit with correct border, align and anchor as well so I don't have to hack again by giving additional BorderSpacing for the button to be visible in certain cases.

[Bart]

Bart, GitHub uses Git, not Mercurial. I don't know how you managed to use SVN with a Git repo.

This worked. The link to the website he provided offered this option, so I just tried.

Code: [Select]

C:\Users\Bart\LazarusProjecten\derit>svn co https://github.com/derit/
ButtonEdit/trunk ButtonEdit


If if breaks temporarily, I don't care. It is time to get this component fixed.

It'll break building Lazarus ATM, which would complicate developing this component IMO.
I would propose to develop (inside Lazarus trunk) TButtonEdit.
This way it is easier for all interested parties to contribute.
Currently the code is not mature enough to replace TEditButton.

When done, we could move to the new TButtonEdit and mark TEditButton as deprecated.
(In theory this could possibly be done with some simple search/replace over Lazarus source code...?)

Bart

[JuhaManninen]

Would you please guide him? There might be little communication problem due to his english, but that's just a small problem. The bigger one lies in his experience in version control system (especially writing a patch) and understanding LCL.

Yes, we are having e-mail communication. And yes, there are problems caused by English language and by a version control system.
Initially his code looked very good and I thought he knows version control, too.

I could rename the component and place it to Lazarus trunk myself tomorrow. First I must test that it compiles and mostly works. I will write to derit and agree with him tomorrow.
Bart, after that you can maybe apply your changes somehow. They cannot be applied directly though.
And don't worry, the trunk must compile after every change. Only some functionality can temporarily be broken or missing.

[Bart]

... after that you can maybe apply your changes somehow. They cannot be applied directly though.

Why not?
For starters, you cannot have property Text set Button.Caption as it is now.

And don't worry, the trunk must compile after every change. Only some functionality can temporarily be broken or missing.

In current state it will not compile (that is: when you replace current TEditButton's implementation with TButtonEdit's one).

Bart

[Derit]

Hi,

I tried to checkout your repo using EasyMercurial, but it failed.
Luckily I could use svn to check it out.

I propose some changes.

• Visually I made it so that it does not look like the button is inside the edit
• Property Color now only sets/gest color of the TEdit
• I used a more common method to set initial width/height
• Property Text now reflects text in edit
• Introduced property ButtonCaption

See attached diff.

Please comment.

Bart

Thanks Suggestion Bart

[Derit]

Bart, GitHub uses Git, not Mercurial. I don't know how you managed to use SVN with a Git repo.

I use Turtoise Svn ....

[Bart]

I would suggest doing something like this for the transition:

Code: [Select]

unit EditBtn; {$ifdef somedefine} deprecated{$endif}

uses
  {$ifdef somedefine}
  Buttonedit
  {$else}
  ....
  {$endif}
  ;


Interface

type
{$ifdef somedefine}
  TCustomEditButton = ButtonEdit.TCustomButtonEdit;
  TEditButton = ButtonEdit.TBttonEdit;
  TFileNameEdit = ButtonEdit.TFileNameEdit;
  ...
{$else}
  //curent interface
{$endif}

implementation

{$ifNdef somedefine}
//current implementation
{$endif}

end.

Bart

[Bart]

Derit, I see that you have a tendency to apply properties (like Cursor, AutoSize, PopupMenu) to the button only.
Maybe you should rethink that.

At least AutoSize should apply to the whole control IMO.
There may be a necessity to split some properties (like EditCursor, ButtonCursor), but that would increasingly become rather cumbersome.

B.t.w. what license is your code, I assume LGPL like the LCL? You mention "See the file COPYING.modifiedLGPL.txt, included in this distribution", but that file is not in your repo.

Bart

[JuhaManninen]

I would suggest doing something like this for the transition:
...

No, I have already done that once and nobody bothered to look at it. Maybe nobody even noticed it is there.
The previous version was there for over a year and anybody could have tested it with NewEditButton define. The events did not work very well but it makes no difference if nobody looks at it.
Now I will either apply it without IFDEFs or then not apply at all.

About the license, all components in LCL are LGPL and Derit has promised to give his component for LCL. It is LGPL then.

I planned to look at the component tonight and maybe apply it but I had no time. It will happen during the weekend...

[Bart]

See attached diff.

Bart

[Bart]

I would suggest doing something like this for the transition:
...

No, I have already done that once and nobody bothered to look at it. Maybe nobody even noticed it is there.

Well, I didn't notice...

I think this is too big a change to commit at once.
For starters the published properties won't match (they cannot, by design), so even if you can compile the IDE it will crash upon start.
I would like to have the chance to develop this in a more relaxed mannor and time frame, not rushing it.

If you follow my proposal (or just develop it as a separate control, which may in time replace the old one), I will follow up on it and implement it until it either functions well, or turns out to be a dead end.
So you have my commitment as developer.
Derit can still provide patches.

Bart

[Derit]

Well, I didn't notice...

I think this is too big a change to commit at once.
For starters the published properties won't match (they cannot, by design), so even if you can compile the IDE it will crash upon start.
I would like to have the chance to develop this in a more relaxed mannor and time frame, not rushing it.

If you follow my proposal (or just develop it as a separate control, which may in time replace the old one), I will follow up on it and implement it until it either functions well, or turns out to be a dead end.
So you have my commitment as developer.
Derit can still provide patches.

Bart

many changes to it ....
add deprecated to editbutton old one

[Bart]

Personally I forsee many problems because of the different published properties.
This would mean that any Lazarus programm using current TEditButton (descendant) will crash upon loading once we ditch the old implementation. It is unavoidable if we proveed like this.

A better way might be to just write this component, make it better than the old one and then manually replace it where we/the users want it to.
Let the old implementation intact and mark it deprecated once the new one is up and about.

This way we can (right now) decide to add this component to LCL and develop it there (would be my choice).
It could go on a new Tab: "Grouped Controls" (IIRC MAttias invented this name?).
For me now it's a PITA, since many of my proposed changes conflict with derit's work.

I really don't want to (have to) change everything back again, looking at his code now (rev. .
@Derit: this is not personally, I rather feel some development decisions regarding this component should be changed, and better do it now than later.

Bart

[JuhaManninen]

Personally I forsee many problems because of the different published properties.

Yes, uhh!
Now I looked properly the recent development of derit's ButtonEdit. It has gone bad.
Last time I looked was the initial version when he started this thread.
It is the 2. commit in the repository, titled: "New Repo", SHA1: e0f751a0f5eba00b65f

What pulled my attention was this in constructor:

  with FEdit do
  begin
    Align := alClient;
    BorderStyle := bsNone;
    Parent := Self;
    ParentColor := True;
    OnChange := @DoEditTextChange;
    OnClick := @DoEditClick;
    OnExit := @DoEditExit;
    OnEnter := @DoEditEnter;
    OnKeyDown := @DoEditKeyDown;
    OnKeyPress := @DoEditKeyPress;
    OnKeyUp := @DoEditKeyUp;
    OnMouseDown := @DoEditMouseDown;
    OnMouseUp := @DoEditMouseUp;
    OnMouseEnter := @DoEditEnter;
    OnMouseLeave := @DoEditMouseLeave;
    OnMouseMove := @DoEditMouseMove;
  end;

and how the Do... methods and the events were done. (The Do... methods should not be virtual though).
I thought, this guy must be clever. He got this right while others have failed.
Even his initial naming "ButtonedEdit" was superior to our current EditButton. (I think we should use the better name and make EditButton a deprecated alias).

After that things started to go wrong for some reason. Working with Lazarus sources did not work well, and this separate component has become worse. Now it is as bad as the Blaazen's version (sorry Blaazen).

I think I under-estimated the difficulty of this task. Sorry everybody about that. Now I see 2 ways to go:

1. I make the component based on commit "New Repo", SHA1: e0f751a0f5eba00b65f.
I can put it inside an IFDEF if really needed.

2. Bart has now a vision of this component and energy to implement it. I suggest Bart takes charge on the issue.
You can use the "New Repo" commit or any other code as a base.
Using IFDEFs is ok. Just don't make a completely separate component because we really need a replacement for the current buggy TEditButton.