I had some time to spare and I thought I might take a shot at, my long time desire of, getting rid of 'with's in VirtualTrees.pas

One thing I noticed is this: If the with block is more involved than a simple element, such as this:

       with Owner, FHeader, FFixedAreaConstraints, TreeView do begin

you have to start 'exploding' from the right-most element and work your way to the left-most one. Otherwise, the resulting code can become meaningless/uncompilable.

Since there's that workaround, I can live with that caveat.

But, I think the one below is a bug:

Here is the original code:

procedure TVirtualTreeColumns.UpdatePositions(Force: Boolean = False);
var
   I, RunningPos: Integer;
begin
   if not FNeedPositionsFix and (Force or (UpdateCount = 0)) then begin
     RunningPos := 0;
     for I := 0 to High(FPositionToIndex) do
*with Items[FPositionToIndex[I]] do **begin*
         FPosition := I;
         FLeft := RunningPos;
         if coVisible in FOptions then Inc(RunningPos, FWidth);
*end*;
   end;
end;

when I explode 'Items[FPositionToIndex[I]]' it becomes this:

procedure TVirtualTreeColumns.UpdatePositions(Force: Boolean = False);
var
   I, RunningPos: Integer;
begin
   if not FNeedPositionsFix and (Force or (UpdateCount = 0)) then begin
     RunningPos := 0;
*for I := 0 to High(FPositionToIndex) do**
**      Items[FPositionToIndex[I]].FPosition := I;*
       Items[FPositionToIndex[I]].FLeft := RunningPos;
       if coVisible in Items[FPositionToIndex[I]].FOptions then
Inc(RunningPos, Items[FPositionToIndex[I]].FWidth);
   end;
end;

At first sight, it looks OK. It compiles.

But, it has, silently, intoduced a bug: It has removed the 'begin end' block that belonged to the 'with' and did not shift it back to the 'for' loop.

IOW, it should have been like this. Notice that 'begin' moved to back to the for-loop block.

procedure TVirtualTreeColumns.UpdatePositions(Force: Boolean = False);
var
   I, RunningPos: Integer;
begin
   if not FNeedPositionsFix and (Force or (UpdateCount = 0)) then begin
     RunningPos := 0;
*for I := 0 to High(FPositionToIndex) do begin*
       Items[FPositionToIndex[I]].FPosition := I;
       Items[FPositionToIndex[I]].FLeft := RunningPos;
       if coVisible in Items[FPositionToIndex[I]].FOptions then
Inc(RunningPos, Items[FPositionToIndex[I]].FWidth);
*end*;
   end;
end;

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20140608/f7568514/attachment.html>

Yes. That's why the tool is in Cody and not in the standard IDE.

Fixed.

Mattias

Could an alternative method of 'with explosion' be introduced so that when we place the cursor on the 'with' (rather than the element we want to explode away), cody decides on the best approach (i.e. start with the right-most one).

Thank you.

Does that also apply to other forms of 'hanging with blocks' such as:

if something then
   with somethingelse begin
     {code}
   end;

If so, these 2 (including the right-most issue) should be the last one(s).

I can say that with some confidence because I have just finished getting rid of ALL 'with's from VirtualTrees.pas in less than half an hour.

And, it is all thanks to cody, because I have been trying (on and off) doing that for the last 6-7 years (became something of an obsession to me).

How I wish something like that was available for Delphi the paid-through-the-nose IDE..

[...] Does that also apply to other forms of 'hanging with blocks'

Yes.

No.
You need a 'do'.

:)

Mattias

:) I'd have been surprised if your hadn't caught that typo.

> And, it is all thanks to cody, because I have been trying (on and off) > doing that for the last 6-7 years (became something of an obsession to me). :)

I've got a couple of feature requests, such as the ones below:

1) Could there be an option so that cody leaves (within the source itself) a commented-out copy of the block it has altered.

This is needed for 2 reasons.

a) I have noticed that cody saves a copy of the altered file to disk immediately. I am not sure if this is the intended behavior; but if it is by design, a record of what it has altered might be desirable. Even though 'undo' can revert back the changes, there may be times you don't want to revert everything but go through the cahnges and correct them manually.

b) I also noticed that cody works within the active compiler switches ($IFDEF stuff). While this is good, it also means it leaves out the code in the '$ELSE' part. This may/will become problematic when the '$ELSE' becomes active but there's no way to 'explode with' for it.

2) It would be immensely more useful if it could do its thing on unit-wide, class-wide and function/procedure-wide.

Where can I write these requests so that they don't get forgotten?

[...] 1) Could there be an option so that cody leaves (within the source itself) a commented-out copy of the block it has altered.

Feel free to send a patch for that.

This is needed for 2 reasons. a) I have noticed that cody saves a copy of the altered file to disk immediately.

Huh? Not that I know.

Changing the $ELSE part is dangerous, because the context is wrong. The parser does not parse structures within $ELSE parts (e.g. with). It should warn the user to check carefully those parts.

2) It would be immensely more useful if it could do its thing on unit-wide, class-wide and function/procedure-wide. Where can I write these requests so that they don't get forgotten?

http://wiki.freepascal.org/Feature_Ideas

Mattias

I haven't looked at the code but --from what it alredy does-- it looks beyond me.

I checked again. You're right it doesn't do that.

Warnings would, of course, be useful.

But, for a complex block it will not really help.

This is an example of what I mean:

with something do begin
{$IFDEF FAVORITE_OS}
{a lot of lines}
with somethingelse do begin
         {a lot of lines}
       end;
     {a lot of lines}
   {$ELSE}
{a lot of lines}
with somethingelse do begin
         {a lot of lines}
       end;
     {a lot of lines}
   {$ENDIF}
end;

At the moment, cody does handle all the complexity for the active half of that IFDEF but when the ELSE part is active, since we will have lost the encapsulating 'with', we will have to copy&paste that from an original.

Obvioulsy, with careful planning it could be done, but there's always the possibility for an oversight which would result in uncompilable/orphaned code.

I am not, mind you, saying my idea is the best one (I don't even know if it could be implemented); I am merely trying to highlight a potetial pitfall.

How about shifting a copy of the 'with' line to inside these IFDEF's, such as:

{$IFDEF FAVORITE_OS}
{code which 'with's have been exploded.}
   {$ELSE}
with something do begin
{a lot of lines}
with somethingelse do begin
           {a lot of lines}
         end;
       {a lot of lines}
     end;
   {$ENDIF}

IOW, when the time comes for the ELSE part, all the information is there and it compiles even without exploding the 'with's.

Yes, that's a good example. somethingelse can depend on the OS. You need the context to do it right.

Good idea. It is not always correct, but it would be better than nothing (the current solution).

Mattias

I implemented that.

Mattias

Mattias Gaertner wrote

I implemented that.

Thank you.