[Lazarus] "Figures" in code explorer

[Alexander Klenin]

On Sun, Apr 19, 2009 at 22:02, Mattias Gaertner
<nc-gaertnma at netcologne.de> wrote:
>> A few questions/suggestions for improvement:
>> 1) Where did the name came from? It is totally meaningless for me.
> See http://www.askoxford.com/concise_oed/figure?view=uk

Hm. I'll quote that:
> figure, noun:
> 1 a number or numerical symbol.
> 2 an amount of money.
> 3 a person’s bodily shape, especially that of a woman.
> 4 a person seen indistinctly.
> 5 an artistic representation of a human or animal form.
> 6 a shape defined by one or more lines.
> 7 a diagram or illustrative drawing.
> 8 (Music) a short succession of notes producing a single impression.

Not one meaning correspond to 'a hint about possible problem in the code'.

>>   Maybe 'Stylistic notes' or 'Style' or something along these lines?
> Not all are about style.

At least it will give user a vague idea what it is ;-)
How about simply 'hints'?

>> 2) What is wrong with published properties without default?
>
> See
> http://wiki.freepascal.org/User_Changes_2.2.4#Default_property_values
> If a property is streamed without need it
> 1. increases the size of the lfm
> 2. makes updating error prone, because even useless properties are
> streamed. For example if a property was introduced and later
> removed/renamed it is still in the lfm and the compiler does not notice.
>
> The LCL had the last case a few times in the last years and every time
> you have to check all lfm files. Therefore the LCL has a built in list
> of ignored properties.

These are all good arguments, but _against_ the removal of 'default default'
property values. IMHO the FPC's team decision is wrong, but I think it
is too late ;-(
Now I have to to bloat my code with 'default false' lines, which were not
necessary before.
I see the reason for the hint now -- but I think it should be made a
FPC warning then.

>> 3b) Maybe option 'parameters', lice line/procedure counts, should be
>> placed near the corresponding checkboxes?
>> 3c) What is the reason for 'search char constants' option? I think it
>> should just be always on.
>
> IMO char constants like #10, #13, 'a' or ' ' don't need to be
> listed.

Maybe, but how about #34 or 'ё'? Char constant may be just as magical
as any other ;)

>> 3d) I think '' (empty string) should be added to the default list of
>> ignored constants.
>
> IMO no one wants to see empty string constants in this list, so no
> need for an option.

But it _is_ shown now.

>> 4b) Add 'Unsorted procedures' similar to 'unsorted members'
>
> I rarely have seen units where all procedure are sorted alphabetically.
> Same for enums, types, variables and constants.

Well, same for methods and properties.
Does LCL have one unit where 'unsorted members' list is empty?
Still, this list is IMO useful and so it is for procedures.

>> 4c) Add 'Empty begin..end' block, probably with the same check for the
>> comments as in (4a)
>> 4d) Add 'Single statement in begin..end block'
>> 4e) Add 'Too many nesting statements' and 'Too many nesting
>> procedures'
>> 4f) Add 'Line loo long' with default limit of perhaps 80
>> or 90 characters.
>> 4g) Add 'Incorrect indentation' for the cases where
>> indentation does not correctly represent
>>   statement nesting.
>
> Are you joking or are you a grammar nazi?

Well, 'grammar nazi' can not exist for a compiled language, since
compiler will simply reject grammatically incorrect source.
So you probably mean 'stylistic nazi'. Well, yes, actually I am ;-)

However, the checks I suggested are very far from 'nazi' level.
Truly nazi-checks would include things as 'no double empty lines',
'all local variables start from lower case', 'begin statement
never/always alone on the line',
'no underscores in identifiers', '"try.. finally..Free..end" for each
locally created object'
and many, many others.

The ones I suggested are in fact very conservative and some are in
direct analogue with already existing ones:
"Empty begin..end" <=> "Empty class section"
"Too many nesting levels" <=> "Too long procedure/Too many nested procedures".

"Incorrect indentation" is IMO hard to implement, but it will probably
be the most useful
of all checks, since it will with 99% probability indicate an actual
bug (even if merely the code formatting bug).

>> 4h) Add 'Fixme' similar to 'TODO'
> AFAIK the todo list is not yet customizable. If you extend it, the code
> explorer will be extended too.

Fair enough.

>> 'TODO' is only recognized inside multi-line {} comments,
>>   not inside single-line (//) ones.
>Fixed.

Thanks.

-- 
Alexander S. Klenin