Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / lazarus / 2009-04

Code Observer notes

7 messages · 3 people ·

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Alexander Klenin

1) The name is good, if somewhat reminiscent of market-speak. "Buy our Lazarus -- now with Code Observer (tm)" ;-)

2) The body of empty procedure is superfluously detected as empty block.

3) Perhaps 'Create' and 'CreateFmt' should be added to the list of functions ignoring constants?
raise Exception.Create('Error text') is a common construct.

4) I still think ' ' (space) should be added to the list of ignored constants -- for those who prefer to turn single-char constant detection on.

5) 'Show Code observer' group box should better be named 'Show observations'

6) Some kind of sorting is necessary for the observations in the box. Perhaps at least alphabetical?

Alexander S. Klenin

Reply by email

Paul Ishenin
to Alexander Klenin
Alexander Klenin wrote
3) Perhaps 'Create' and 'CreateFmt' should be added to the list of functions ignoring constants? raise Exception.Create('Error text') is a common construct.

No. Create and CreateFmt strings needs to be translatable even for exceptions.

Best regards,
Paul Ishenin.

Reply by email

Alexander Klenin
to Paul Ishenin
Paul Ishenin wrote 7 lines
Alexander Klenin wrote: > 3) Perhaps 'Create' and 'CreateFmt' should be added to the list of > functions ignoring constants? > ? raise Exception.Create('Error text') is a common construct. No. Create and CreateFmt strings needs to be translatable even for exceptions.

This is true, but unrelated.
First, naming a string constant does not make it translatable, it should be made resourcestring (so perhaps it should be a separate observation --
"message is not translatable", where "message" is defined as string constant containing at least one word)
Second, there are different means of i18n, and some of them, like dxgettext, do not depend on resourcestrings.
Third, this same argument can be applied to Fromat, write/writeln and other functions already present on ignore list.

Alexander S. Klenin

Reply by email

Paul Ishenin
to Alexander Klenin
Alexander Klenin wrote 11 lines
This is true, but unrelated. First, naming a string constant does not make it translatable, it should be made resourcestring (so perhaps it should be a separate observation -- "message is not translatable", where "message" is defined as string constant containing at least one word) Second, there are different means of i18n, and some of them, like dxgettext, do not depend on resourcestrings. Third, this same argument can be applied to Fromat, write/writeln and other functions already present on ignore list.

1. Later sring and numeric consts will be split between 2 parent nodes.
2. LCLStrConsts and LazarusIDEStrConsts unit uses resourcestrings and therefore this is default way to translate strings.

And about WriteLn, Write - usually we are using them in IDE for debug purposes only. But I agree that they should not be present in this list by default.

On the other handle, this list is adjustable so just feel your own list with desired functions :)

Best regards,
Paul Ishenin.

Reply by email

Mattias Gaertner
to Alexander Klenin: 1) The name is good, if somewhat reminiscent of market-speak. "Buy our Lazarus -- now…
Show quoted text 5 lines
1) The name is good, if somewhat reminiscent of market-speak. "Buy our Lazarus -- now with Code Observer (tm)" ;-) 2) The body of empty procedure is superfluously detected as empty block.

empty procedures can contain comments.
I don't see a problem with listing a completely empty block in two categories.

3) Perhaps 'Create' and 'CreateFmt' should be added to the list of functions ignoring constants? raise Exception.Create('Error text') is a common construct.

As Paul said, these are normal texts and are therefore not in the default list. If you use them otherwise you can add them to your personal list.

4) I still think ' ' (space) should be added to the list of ignored constants -- for those who prefer to turn single-char constant detection on.

Why ' ' and not #10 or #13 or #0?

Show quoted text 5 lines
5) 'Show Code observer' group box should better be named 'Show observations' 6) Some kind of sorting is necessary for the observations in the box. Perhaps at least alphabetical?

A patch is welcome.

Mattias

Reply by email

Alexander Klenin
to Mattias Gaertner
Mattias Gaertner wrote 5 lines
> 4) I still think ' ' (space) should be added to the list of ignored > constants -- for those who prefer to turn single-char constant > detection on. Why ' ' and not #10 or #13 or #0?

It is by far the most common. Also, unnamed #10 and #13 might indicate platform-dependent line endings usage.

Show quoted text 7 lines
> 5) 'Show Code observer' group box should better be named 'Show > observations' > > 6) Some kind of sorting is necessary for the observations in the box. > Perhaps at least alphabetical? A patch is welcome.

http://bugs.freepascal.org/view.php?id=13551

BTW, can you please also review http://bugs.freepascal.org/view.php?id=13501 ? It followed from our previous discussion.

Alexander S. Klenin

Reply by email

Mattias Gaertner
to Alexander Klenin
Show quoted text 9 lines
On Tue, Apr 21, 2009 at 19:45, Mattias Gaertner wrote: >> 4) I still think ' ' (space) should be added to the list of ignored >> constants -- for those who prefer to turn single-char constant >> detection on. > > Why ' ' and not #10 or #13 or #0? It is by far the most common.

I doubt that:

lazarus sources:
#10: 911
#13: 1425
#0: 1125
' ': 1740
Without debug calls there are only about 1000 ' '.

fpc sources:
#10: 1213
#13: 5766
#0: 2384
' ': 1846
Without debug calls there are only about 1500 ' '.

Also, unnamed #10 and #13 might indicate platform-dependent line endings usage.

And ' ' might indicate i18n problems.

Show quoted text 13 lines
>> 5) 'Show Code observer' group box should better be named 'Show >> observations' >> >> 6) Some kind of sorting is necessary for the observations in the >> box. Perhaps at least alphabetical? > > A patch is welcome. http://bugs.freepascal.org/view.php?id=13551 BTW, can you please also review http://bugs.freepascal.org/view.php?id=13501 ? It followed from our previous discussion.

I will take a look.

Mattias

Reply by email