On 02/06/2014 11:48 PM, Dick Hollenbeck wrote:
> On 02/06/2014 04:26 PM, Maciej Sumiński wrote:
>
>> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
>> to leave it as it is now, particularly that it is saved as a 32-bit number in
>> .kicad_pcb files and I would like to preserve compatibility (so if older versions
>> expect 32-bit number there
>
> Note, that if we were always bound to allow older versions of KiCad to load newer files,
> then it would be impossible to add new features which needed saving on disk.
>
> I value new features far more.  We can all stop writing code and wasting time on a mailing
> list if we are not to write features requiring file format changes.  It would essentially
> mean that we could only make improvements which do not require change.
>
> The pay is too low already.  If you can find a way to make improvements without also
> making changes, then I am in the wrong universe.

Sure, I am aware that to improve you need to change, but not always the 
file format. In this case, one cannot even benefit from saving the 
visibility settings for netname layers, as they are turned on/off 
together with copper layers. To take advantage of that, we need to add 
some checkboxes first.
We could put in a document all the changes that we want to introduce to 
the file format and then change it every e.g. 6 months according to the 
paper. In this way, users will not be bothered by so frequent file 
format changes. Especially that the majority seems to be using the 
stable version, so they would not be able to open files done with the 
newest product (and getting the freshest product is still beyond 
capabilities of many users).

>> - let them have it and do not crash/do weird things on load
>> of PCBs holding 64 bits for elements visiblity).
>
>
> Not really an issue, if you parse an ASCII 64 bit number into a 32 bit binary integer, in
> old code, the lower 32 bits are likely preserved.  So in this case you might have been OK.
>   If not, folks can stick with their old software and old BOARDs.

Ok, I checked the code and it probably will not crash. parseHex() calls 
strtol(), so depending on the platform (and size of long int) you will 
get either the least significant 32 bits (good; that works for x86_64) 
or LONG_MAX or LONG_MIN (i686) - to me it looks like a bug.

>> Saving netnames layers visibility is
>> not really crucial, it can be restored basing on visibility settings of appropriate
>> copper layers.
>
> I like the feature!  And I like the 33rd feature which I want to save to disk, whatever it is.
>
> I would like to save it to disk, and have it be BOARD centric, not project centric.
>
> Eventually we'll hit some kind of wall, if not here, now, then sometime.
> Saving a bit map as an ASCII integer is only one possibility anyways.  There are numerous
> alternatives for expansion:
>
> a) save a 64 bit integer in ASCII.
>
> b) use s-expressions to name each bit.
>
> c) An arbitrartily long ASCII binary number
> 0000_1011_1110_0011....
>
> This can extend far off the page if needed.
>
> Parsing that would be pretty easy, the underscore saves a human some counting.  At some
> point you quietly switch to a bitmap of arbitrary length.  But since you've hidden the
> implementation from the client code, that is nobody's damn business.
>
> d) blah blah blah
>
>
>
>> The change from int to long resulted only from the fact that in the back of my head I
>> remember that int is guaranteed to be 16 bits,
>
> Fully incomprehensible.   I don't know where 16 bits even enters into it.  Find a computer
> that uses 16 bits and you've found one that should not be running KiCad.
>
>
> The lesson here is that "long" is
>
> a) not necessarily longer than int, and
>
> b) not known until you pick a machine and a compiler.  So you don't get 64 bitness if
> thats what you need.

But it is not what I needed - as I said, it was only to assure 32 bits 
size. I have changed this to long, as long is guaranteed in the standard 
to have 32 bits. I admit that on some platforms I could waste 4 bytes of 
RAM. But if int does the job on the supported platforms then why not 
leave it as it is (or change it to int32_t)?

>> so maybe on some platforms the code
>> would not run as expected. But as it works on the major platforms then I simply do not
>> insist on changing that.
>
> int64_t is 64 bits, guaranteed.
>
>
> Anyway - the patch fixes the problem even when
>> m_VisibleElements stays as integer, so what do you think about the rest of the
>> changes?
>
> I have not looked at it, I can if nobody else want's to.  The original patch came to me
> via email.
>
>
>
>> Regards, Orson

Sincerely, my main concern is to fix the bug. I have already learned a 
lesson and reverted the commit that you have found improper. Is there 
anything else that I could do to have the branch merged?

Regards,
Orson