On 11/21/2013 01:36 PM, Dick Hollenbeck wrote: > On 11/21/2013 09:13 AM, Wayne Stambaugh wrote: >> 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. > > > No thanks to nag warnings on footprint saves. Jean-Pierre and I both sort of > worked on > the current plugin solution. (Eeschema is different, it is usage by > reference so a rename > has larger consequences.) > > > In this case, I think its easiest just to rename illegal footprints on load, > otherwise I > would not have wasted 15 minutes saying so in my last email. If it bothers > you, make it a > PLUGIN option and teach LEGACY_PLUGIN to do it only in the presence of a > particular > option/property during FootprintLoad() or FootprintEnumerate(). It does not > bother me. > > > Carl could tune his python script with the option set when he converts > everything over to > github and pretty. But this is needless work in my view. The KiCad > footprint libraries > are rarely something I use in any case, probably less than 10% of any given > board. A > footprint rename would not spook me or any one else IMO. > > I think I will be using mostly the Eagle ones moving forward, and I have no > prior > expectation on how footprints should be spelled, so a rename there is > tantamount to the > original author assigning the same name that we assign to it. Or I use > footprints I have > made. > > Whatever the footprint naming policy is, I would think of it as "KiCad wide", > not specific > to pretty format. > > "Yes" to nagging on keyboard entry of new illegal footprint names at any UI > prompt. > > > >> 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. > > That is not what I see, unless you decide to use 8 bit strings in the current > locale. And > I have had a reversal of opinion on that. I think 8 bit strings in KiCad > might be better > held in UTF8 format in any case. This makes the new wxString constructor not > useable: > > wxString( const char* ) > wxString( std::string ) > > since these both convert from current locale encoding, not explicitly UTF8. > > (And although current locale on linux is typically UTF8, you cannot assume > this.) > > So it will take a lot of augmentation to come up with a sound future strategy. > > I would recommend our
own > class derived from wxString() so we can override the above two > constructors, but still pass them around to wx APIs as if they were really > wxStrings. > > We could do that now, even with wx 2.8, and get on with it. > > > Dick > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
