kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

On 11/21/2013 9:25 AM, Dick Hollenbeck wrote:
> 
>>>> Using path separators in module names was not an issue in the legacy
>>>> file format.  With the pretty format, it will cause the file path to be
>>>> incorrect.  I have it on my todo list to add filtering to prevent new
>>>> modules names from containing illegal file name and path characters.  I
>>>> will have to add this test when converting other library formats to the
>>>> pretty format.  My guess is this is what happened in this case.  For
>>>> now, we will have the fix them by hand as we come across them.
>>>> Hopefully there wont be too many of them.
>>>
>>>
>>> We ran into that in the EAGPLE_PLUGIN.  The simplest solution was to simply lie about the
>>> name encountered when loading the footprint.
>>>
>>> This is done in function
>>>
>>> static bool fix_eagle_package_name( string* aName )
>>>
>>> in the eagle plugin.  But it now looks like the strategy could have general applicability,
>>> including in LEGACY_PLUGIN.
>>
>> It might be a good idea to make this public for use in other plugins
>> when attempting to save pretty libraries.
> 
> 
> 
> As to its use, I wish to add clarification since the last part of your sentence confounds
> me a little.  The name change happens during the eagle plugin's footprint name caching
> operation.  So it is a modification during loading, i.e. *reading* (not saving) of a
> library containing potentially "illegal" footprint names.
> 
> It is only indirectly tied to "saving pretty libraries", and here's how:  Since the all
> names are legal after loading, there is nothing special to do for pretty save, its already
> been done, all names in RAM are legal already.
> 
> Some plugins hold footprints in individual files, others do not.  Only for the latter
> case, there is potential for an illegal filename problem. That's where a footprint name
> translation function was used, in EAGLE_PLUGIN.  But we see the same situation now in
> LEGACY, not sure about Geda.  Pretty code at most might only get an assert, but it would
> not be necessary to change anything there.

This would apply to any library plugin that stores multiple footprints
in a single file.  Although it might be confusing to the user to see
that a footprint name with an "illegal" file name characters has been
changed unexpectedly.  It might be more prudent to check for illegal
names and warn the user that footprint will be renamed when saving to
the pretty format rather than changing them at load time.  Geda is like
pretty in that each footprint is stored in a separate file.

> 
> EAGLE_PLUGIN starts with 8 bit strings, and the name conversion when it happens is an
> exception, not the norm for speed reasons.  The "fix eagle package name" function I have
> works on 8 bit strings, I don't know if it is useful to a plugin working with puffy
> wxString footprint names.  If so, it could be sensible to make it a static member function
> of PLUGIN.
> 
> Obviously naming policy needs to be focused into fewest control points.  Also, we're
> implicitly assuming that any legal base filename is also a legal footprint name, and that
> assumption needs verification.

I don't recall ever seeing anything that would limit the characters used
for footprint names in Pcbnew but then again I've never really looked at
it that closely.  It probably makes sense to enforce the pretty naming
policy even when creating new footprints in legacy libraries to prevent
issues when converting them to the pretty format.  That only leaves
verifying legacy and third party libraries for illegal names when
converting to the pretty format.

> 
> 
> (BTW, I admit to be coming down with an ever increasing distaste for wxString.  I don't
> like broccoli anymore either, and yet it is good for me.  wxString is only good for me to
> the extend that is eases use of wxWidgets.  I don't see where it makes programming any
> easier however.)

This issue should go away for the most part once wxWidgets 3 becomes the
minimum requirement to build KiCad.

> 
> 
> Dick
> 
> 
>>
>>>
>>> The advertised names are converted before even using them, from FootprintLoad() or cacheLib().
>>>
>>> This is the chosen path for EAGLE anyways.
>>>
>>> Note that it was not good enough to do a character to character swap.  That lead to a
>>> duplicate footprint name.  No foolin', murphy is always in charge.
>>>
>>>
>>>    We were swapping '/'  with say '-'.
>>>
>>>
>>> But then ran into a library with say both  'XYZ/' and 'XYZ-' in the same library!
>>
>> Nobody ever said this was going to be easy :)
>>
>>>
>>> Back to the drawing board.  What's now in place is a URL encoding like substitution.
>>> Nobody would ever name their footprint 'XYZ%2f'  would they?
>>
>> It seems like it would be safe but I wouldn't bet on it.  There is
>> always that one person who will confound your logic.
>>
>> Wayne
>>
>>>
>>> Please say no, because finding a unique name is the goal.  After one read / write cycle, a
>>> person can always use the module editor to rename anything with a % in it.  Or in the case
>>> of pretty, just a file manager.
>>>
>>>
>>> Dick
>>>
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>