Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-12 Thread Mattias Gaertner via Lazarus
On Tue, 12 Dec 2017 11:14:09 +0100
Sven Barth via Lazarus  wrote:

>[...]
> Patches with formatting changes are very likely to be rejected in code that
> belongs to the FPC project (can't speak about Lazarus here) and for the
> compiler itself it is even nearly assured.

Lazarus loosely follows the Delphi coding style. Patches that changes
formatting to something else are usually rejected.

Mattias
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-12 Thread Marco van de Voort via Lazarus
On Tue, Dec 12, 2017 at 11:14:09AM +0100, Sven Barth via Lazarus wrote:
> want.
> The modus operandi is to adhere to the formatting of the surrounding code.
> 
> I will do whole work again as Romans want if Romans pay me.
> 
> 
> Patches with formatting changes are very likely to be rejected in code that
> belongs to the FPC project (can't speak about Lazarus here) and for the
> compiler itself it is even nearly assured.

I guess it is the same as with FPC, if there is hope on some nugget and the
mess is not too bad, then somebody might trawl through it. But the files I 
examined
had hundreds of formatting hunks and very minimal and obvious changes/fixes.
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-12 Thread Sven Barth via Lazarus
Am 12.12.2017 09:07 schrieb "Sergey Bodrov via Lazarus" <
lazarus@lists.lazarus-ide.org>:



2017-12-11 19:18 GMT+03:00 Sven Barth via Lazarus <
lazarus@lists.lazarus-ide.org>:

Problem is that I first change code formatting just to make it readable,
> and then do fixes. Most fixes was impossible without reformatting.
>
>
> In projects like Lazarus and Free Pascal it's "when in Rome, do as the
> Romans do" regarding formatting. So don't change formatting only because
> *you* can't work with it. And I highly doubt that the possibility of fixes
> depends on the formatting...
>

http://wiki.freepascal.org/Coding_style - "There are no formal standards"
for FCL and other packages distributed with FPC. So I use Lazarus and
Embarcadero coding guides.


No formal standard does not mean that you can change code formatting as you
want.
The modus operandi is to adhere to the formatting of the surrounding code.

I will do whole work again as Romans want if Romans pay me.


Patches with formatting changes are very likely to be rejected in code that
belongs to the FPC project (can't speak about Lazarus here) and for the
compiler itself it is even nearly assured.

Regards,
Sven
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-12 Thread Sergey Bodrov via Lazarus
2017-12-11 19:18 GMT+03:00 Sven Barth via Lazarus <
lazarus@lists.lazarus-ide.org>:

Problem is that I first change code formatting just to make it readable,
> and then do fixes. Most fixes was impossible without reformatting.
>
>
> In projects like Lazarus and Free Pascal it's "when in Rome, do as the
> Romans do" regarding formatting. So don't change formatting only because
> *you* can't work with it. And I highly doubt that the possibility of fixes
> depends on the formatting...
>

http://wiki.freepascal.org/Coding_style - "There are no formal standards"
for FCL and other packages distributed with FPC. So I use Lazarus and
Embarcadero coding guides.

I will do whole work again as Romans want if Romans pay me.

-- 
*Bodrov Sergey*
software development, IT consulting
http://www.serbod.com
*Phone (Belarus):* +375(25)794-21-58
*Skype:* sergey.bodrov1
*e-mail:* ser...@gmail.com, oxot...@yandex.ru
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-11 Thread Sven Barth via Lazarus
Am 11.12.2017 15:15 schrieb "Sergey Bodrov via Lazarus" <
lazarus@lists.lazarus-ide.org>:



2017-12-11 16:14 GMT+03:00 Marco van de Voort via Lazarus <
lazarus@lists.lazarus-ide.org>:

> On Sun, Dec 10, 2017 at 01:25:59PM +0300, Sergey Bodrov via Lazarus wrote:
> >
> > I change code formatting, many private names, add many comments and
> embrace
> > whole blocks of code into try..finally. And now can't tell, which parts
> is
> > most essential.
>
> Yes. Somebody will have to figure that mess out, preferably by somebody who
> has some recollection about what he has done.  It is always a bad idea to
> do
> big style related operations mixing it with real fixes.  Now you know why
> :-)
>

Problem is that I first change code formatting just to make it readable,
and then do fixes. Most fixes was impossible without reformatting.


In projects like Lazarus and Free Pascal it's "when in Rome, do as the
Romans do" regarding formatting. So don't change formatting only because
*you* can't work with it. And I highly doubt that the possibility of fixes
depends on the formatting...

Regards,
Sven
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-11 Thread Marco van de Voort via Lazarus
On Mon, Dec 11, 2017 at 05:15:36PM +0300, Sergey Bodrov via Lazarus wrote:
> > big style related operations mixing it with real fixes.  Now you know why
> > :-)
> >
> 
> Problem is that I first change code formatting just to make it readable,
> and then do fixes. Most fixes was impossible without reformatting.

If you kept it apart in your VCS, maybe you can backport the fixes then.

But it is nearly impossible to remerge reformatted sources, the number of 
changes suddenly is in the hundreds instead of the signle digits. 

> Maybe you mention file lcid_conv.pas, which contain MS Locale ID to Windows
> Codepage conversion and common for LHelp and chmmaker projects? Yes, chm
> package is a wrong place for it. I don't know, where is better place.

> > So that would need to be cleaned up using FPC 3.0 string support first.
> >
> 
> What is better way to convert WideString to AnsiString without warnings?

There is no way without surpressing warnings locally or use an RTL function
that surpresses warnings. 

But either way, the current way is wrong, even if it surpresses warnings.
The encoding of ansistring is fully runtime defined in 3.0, and code should
respect that as much as possible, while your code just assumes utf8.

> > Naming and linking problems.
> >
> > The units have no name with chm in them, so I see no naming issues and FPC
> > in general does not employ static libraries or so for linking purposes.
> >
> 
> And if that unit names will be same in different packages?

You don't need to have them in a different package. You can just use the
units from package CHM without any downsides. Package CHM only depends on 
other packages that are pure pascal, so it won't pull in dependencies.

If something major comes up, this can be revisited, but I don't see a point
to fragment packages ad infinitum on principle grounds only.
 
> Also, is chm is really needed and used in FPC, without Lasarus?

The textmode IDE also uses CHM, as do various chm file generators like fpdoc
and chmcmd. 

-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-11 Thread Sergey Bodrov via Lazarus
2017-12-11 16:14 GMT+03:00 Marco van de Voort via Lazarus <
lazarus@lists.lazarus-ide.org>:

> On Sun, Dec 10, 2017 at 01:25:59PM +0300, Sergey Bodrov via Lazarus wrote:
> >
> > I change code formatting, many private names, add many comments and
> embrace
> > whole blocks of code into try..finally. And now can't tell, which parts
> is
> > most essential.
>
> Yes. Somebody will have to figure that mess out, preferably by somebody who
> has some recollection about what he has done.  It is always a bad idea to
> do
> big style related operations mixing it with real fixes.  Now you know why
> :-)
>

Problem is that I first change code formatting just to make it readable,
and then do fixes. Most fixes was impossible without reformatting.


> And when I had a quick peak, I scanned two whole files, and didn't find any
> fixes except adding "const" to some parameters, and some 2.6.x era unicode
> fixes that used Lazarus units, and thus can't be merged into FPC repo
> anyway.  (using lazconvencoding or something like that).
>

Hmm, don't remember, that I doing addiction of new includes to 'chm' units.
And encoding-related operations was made in LHelp project, after HTML
content readed from CHM file, but before it passed to IPro HTML viewer.

There was some compiler warnings about WideString -> AnsiString conversion,
that I eliminate with UTF8Encode() from RTL

Maybe you mention file lcid_conv.pas, which contain MS Locale ID to Windows
Codepage conversion and common for LHelp and chmmaker projects? Yes, chm
package is a wrong place for it. I don't know, where is better place.


> So that would need to be cleaned up using FPC 3.0 string support first.
>

What is better way to convert WideString to AnsiString without warnings?

> Naming and linking problems.
>
> The units have no name with chm in them, so I see no naming issues and FPC
> in general does not employ static libraries or so for linking purposes.
>

And if that unit names will be same in different packages?

Also, is chm is really needed and used in FPC, without Lasarus?


-- 
*Bodrov Sergey*
software development, IT consulting
http://www.serbod.com
*Phone (Belarus):* +375(25)794-21-58
*Skype:* sergey.bodrov1
*e-mail:* ser...@gmail.com, oxot...@yandex.ru
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-11 Thread Marco van de Voort via Lazarus
On Sun, Dec 10, 2017 at 01:25:59PM +0300, Sergey Bodrov via Lazarus wrote:
> 
> I change code formatting, many private names, add many comments and embrace
> whole blocks of code into try..finally. And now can't tell, which parts is
> most essential.

Yes. Somebody will have to figure that mess out, preferably by somebody who
has some recollection about what he has done.  It is always a bad idea to do
big style related operations mixing it with real fixes.  Now you know why
:-)
 
And when I had a quick peak, I scanned two whole files, and didn't find any
fixes except adding "const" to some parameters, and some 2.6.x era unicode
fixes that used Lazarus units, and thus can't be merged into FPC repo
anyway.  (using lazconvencoding or something like that).

So that would need to be cleaned up using FPC 3.0 string support first.

> > > 1. FPC package 'chm' contain file fasthtmlparser.pas, that used in some
> > > non-CHM packages and projects. May be better move that file to some
> > generic
> > > package - fcl-base, for example?
> >
> > Afaik this was meant as a copy of that parser dedicated for chm that could
> > evolve with the package.  So if you need it more general, whip up a
> > suitable
> > general version.
> >
> 
> It's just fast and simple SAX-like tags parser, no more. It not rely on any
> external libs/units and not dedicated to CHM.

I looked again, and it is indeed very simple. 

> > What does that make possible that currently is not possible? Do we really
> > have to spin each unit into a separate package? I don't see the point.
> >
> > Note also that iirc CHM lzx and e.g. CAB lzx vary in some ways. (headers?)
> >
> 
> LZX in CHM use non-sliding window, separated to 64K blocks for fast random
> access. It implemented in separate file - paslznonslide.pas
> 
> 
> > But even bypassing that, what is the problem of having to instal 10 extra
> > units (150kb) to make a CAB decompressor?
> 
> Naming and linking problems.

The units have no name with chm in them, so I see no naming issues and FPC
in general does not employ static libraries or so for linking purposes.
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus


Re: [Lazarus] CHM help package (Marco van de Voort)

2017-12-10 Thread Sergey Bodrov via Lazarus
>
> On Fri, Dec 08, 2017 at 03:23:06PM +0300, Sergey Bodrov via Lazarus wrote:
> > There is too many changes in many files, and that can't be posted as
> simple
> > diff patch.
>
> Then layer them. Make patches for the most essential bits, and then
> reallign
> your sources.
>

I change code formatting, many private names, add many comments and embrace
whole blocks of code into try..finally. And now can't tell, which parts is
most essential.


> > 1. FPC package 'chm' contain file fasthtmlparser.pas, that used in some
> > non-CHM packages and projects. May be better move that file to some
> generic
> > package - fcl-base, for example?
>
> Afaik this was meant as a copy of that parser dedicated for chm that could
> evolve with the package.  So if you need it more general, whip up a
> suitable
> general version.
>

It's just fast and simple SAX-like tags parser, no more. It not rely on any
external libs/units and not dedicated to CHM.


> > 2. FPC package 'chm' contain LZX compression routines in some files
> > - paslzx.pas, paslzxcomp.pas, paslznonslide.pas, lzxcompressthread.pas.
> > They not depends on other files in 'chm' and can be maintained as
> separate
> > package.
>
> What does that make possible that currently is not possible? Do we really
> have to spin each unit into a separate package? I don't see the point.
>
> Note also that iirc CHM lzx and e.g. CAB lzx vary in some ways. (headers?)
>

LZX in CHM use non-sliding window, separated to 64K blocks for fast random
access. It implemented in separate file - paslznonslide.pas


> But even bypassing that, what is the problem of having to instal 10 extra
> units (150kb) to make a CAB decompressor?
>

Naming and linking problems.


-- 
*Bodrov Sergey*
software development, IT consulting
http://www.serbod.com
*Phone (Belarus):* +375(25)794-21-58
*Skype:* sergey.bodrov1
*e-mail:* ser...@gmail.com, oxot...@yandex.ru
-- 
___
Lazarus mailing list
Lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus