Re: [Lazarus] Code Observer notes
On Tue, 21 Apr 2009 14:20:55 +1100
Alexander Klenin kle...@gmail.com wrote:
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?
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
___
Lazarus mailing list
Lazarus@lazarus.freepascal.org
http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
Re: [Lazarus] Code Observer notes
On Wed, 22 Apr 2009 01:54:57 +1100 Alexander Klenin kle...@gmail.com wrote: On Tue, Apr 21, 2009 at 19:45, Mattias Gaertner nc-gaert...@netcologne.de 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. 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 ___ Lazarus mailing list Lazarus@lazarus.freepascal.org http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
[Lazarus] Code Observer notes
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
___
Lazarus mailing list
Lazarus@lazarus.freepascal.org
http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
Re: [Lazarus] Code Observer notes
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.
___
Lazarus mailing list
Lazarus@lazarus.freepascal.org
http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
Re: [Lazarus] Code Observer notes
On Tue, Apr 21, 2009 at 14:49, Paul Ishenin i...@kmiac.ru wrote:
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
___
Lazarus mailing list
Lazarus@lazarus.freepascal.org
http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
Re: [Lazarus] Code Observer notes
Alexander Klenin wrote: 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. ___ Lazarus mailing list Lazarus@lazarus.freepascal.org http://www.lazarus.freepascal.org/mailman/listinfo/lazarus
