Re: [Kicad-developers] Handling invalid characters in symbol/component LIB_IDs
On 03/06/2018 07:34 PM, Wayne Stambaugh wrote:
> Orson,
>
> On 3/6/2018 8:55 AM, Maciej Sumiński wrote:
>> I am trying to find a reasonable way to handle symbol and components
>> with invalid characters in their LIB_IDs (see bug report #1752419 [1]).
>> While now it is impossible to create such LIB_IDs, we need to handle
>> documents that had been created before the restriction was introduced.
>>
>> LIB_IDs in SCH_COMPONENTS will have illegal characters replaced on load,
>> but it is not the case for LIB_{PART,ALIAS} classes. Due to that:
>>
>> - Symbol-component links are broken (e.g. component with LIB_ID
>> 'Test/Name' will change to 'Test_Name', but the corresponding LIB_PART
>> will remain 'Test/Name'.
>>
>> - It is impossible to place/edit such symbols.
>>
>> The simplest solution is to process LIB_{PART,ALIAS} LIB_IDs in the same
>> way as id done for SCH_COMPONENTs, so they become valid names that match
>> SCH_COMPONENTs using them (see the attached patches). There are two
>> drawbacks:
>
> I would prefer to keep the behavior consistent with what is done with
> the SCH_COMPONENT LIB_ID renaming. I know it's less than elegant but we
> are going to just have to take our medicine until we transition over to
> the new file format.
That is what I am thinking as well, so I have coded it this way in the
patches.
>> - Changing names that user had previously assigned, which might be
>> annoying if LIB_IDs are used e.g. to create BOMs. Personally I would use
>> the value field that accepts all characters for this purpose.
>
> Can LIB_IDs be used in BOMs? That would be an odd way to do it but I
> suppose you could. Typically the contents of the value field is used
> for BOM output not the symbol name.
>
>>
>> - Naming conflicts may occur (e.g. both 'Test/Name' and 'Test:Name' will
>> be changed to 'Test_Name', but I believe it is a rare case.
>
> I just used a simple counter to append digits to the end of the name in
> the case of name conflicts in the remapping code so a similar approach
> here would not be out of line.
Good point, implemented and pushed to the master branch.
Cheers,
Orson
>> Any thoughts? I believe a more elegant solution will be implemented once
>> the new schematic file format is ready.
>
> The current behavior is just going to last during v5. Once the new
> schematic file format is done and the LIB_ID parser and formatter
> support escaping the / and : characters, we should be able to use any
> character (except for control characters) in LIB_IDs.
>
>>
>> Cheers,
>> Orson
>>
>> 1. https://bugs.launchpad.net/kicad/+bug/1752419
>>
>>
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help : https://help.launchpad.net/ListHelp
>>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp
>
signature.asc
Description: OpenPGP digital signature
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] Handling invalid characters in symbol/component LIB_IDs
Orson,
On 3/6/2018 8:55 AM, Maciej Sumiński wrote:
> I am trying to find a reasonable way to handle symbol and components
> with invalid characters in their LIB_IDs (see bug report #1752419 [1]).
> While now it is impossible to create such LIB_IDs, we need to handle
> documents that had been created before the restriction was introduced.
>
> LIB_IDs in SCH_COMPONENTS will have illegal characters replaced on load,
> but it is not the case for LIB_{PART,ALIAS} classes. Due to that:
>
> - Symbol-component links are broken (e.g. component with LIB_ID
> 'Test/Name' will change to 'Test_Name', but the corresponding LIB_PART
> will remain 'Test/Name'.
>
> - It is impossible to place/edit such symbols.
>
> The simplest solution is to process LIB_{PART,ALIAS} LIB_IDs in the same
> way as id done for SCH_COMPONENTs, so they become valid names that match
> SCH_COMPONENTs using them (see the attached patches). There are two
> drawbacks:
I would prefer to keep the behavior consistent with what is done with
the SCH_COMPONENT LIB_ID renaming. I know it's less than elegant but we
are going to just have to take our medicine until we transition over to
the new file format.
>
> - Changing names that user had previously assigned, which might be
> annoying if LIB_IDs are used e.g. to create BOMs. Personally I would use
> the value field that accepts all characters for this purpose.
Can LIB_IDs be used in BOMs? That would be an odd way to do it but I
suppose you could. Typically the contents of the value field is used
for BOM output not the symbol name.
>
> - Naming conflicts may occur (e.g. both 'Test/Name' and 'Test:Name' will
> be changed to 'Test_Name', but I believe it is a rare case.
I just used a simple counter to append digits to the end of the name in
the case of name conflicts in the remapping code so a similar approach
here would not be out of line.
>
> Any thoughts? I believe a more elegant solution will be implemented once
> the new schematic file format is ready.
The current behavior is just going to last during v5. Once the new
schematic file format is done and the LIB_ID parser and formatter
support escaping the / and : characters, we should be able to use any
character (except for control characters) in LIB_IDs.
>
> Cheers,
> Orson
>
> 1. https://bugs.launchpad.net/kicad/+bug/1752419
>
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] Handling invalid characters in symbol/component LIB_IDs
Attached a small project to observe the problem/test the patches. There
are two libraries inside:
- slash.lib contains one symbol with '/' in its LIB_ID
- slash_conflict.lib contains two symbols that result in a LIB_ID clash
(RELAY_Hongfa_HF115F/012-1H3T and RELAY_Hongfa_HF115F_012-1H3T).
Normally you can use slash.lib, but if you want to check what happens in
case of a conflict, then use the other one.
On 03/06/2018 02:55 PM, Maciej Sumiński wrote:
> I am trying to find a reasonable way to handle symbol and components
> with invalid characters in their LIB_IDs (see bug report #1752419 [1]).
> While now it is impossible to create such LIB_IDs, we need to handle
> documents that had been created before the restriction was introduced.
>
> LIB_IDs in SCH_COMPONENTS will have illegal characters replaced on load,
> but it is not the case for LIB_{PART,ALIAS} classes. Due to that:
>
> - Symbol-component links are broken (e.g. component with LIB_ID
> 'Test/Name' will change to 'Test_Name', but the corresponding LIB_PART
> will remain 'Test/Name'.
>
> - It is impossible to place/edit such symbols.
>
> The simplest solution is to process LIB_{PART,ALIAS} LIB_IDs in the same
> way as id done for SCH_COMPONENTs, so they become valid names that match
> SCH_COMPONENTs using them (see the attached patches). There are two
> drawbacks:
>
> - Changing names that user had previously assigned, which might be
> annoying if LIB_IDs are used e.g. to create BOMs. Personally I would use
> the value field that accepts all characters for this purpose.
>
> - Naming conflicts may occur (e.g. both 'Test/Name' and 'Test:Name' will
> be changed to 'Test_Name', but I believe it is a rare case.
>
> Any thoughts? I believe a more elegant solution will be implemented once
> the new schematic file format is ready.
>
> Cheers,
> Orson
>
> 1. https://bugs.launchpad.net/kicad/+bug/1752419
>
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp
>
slash_test.tar.bz2
Description: BZip2 compressed data
signature.asc
Description: OpenPGP digital signature
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
[Kicad-developers] Handling invalid characters in symbol/component LIB_IDs
I am trying to find a reasonable way to handle symbol and components
with invalid characters in their LIB_IDs (see bug report #1752419 [1]).
While now it is impossible to create such LIB_IDs, we need to handle
documents that had been created before the restriction was introduced.
LIB_IDs in SCH_COMPONENTS will have illegal characters replaced on load,
but it is not the case for LIB_{PART,ALIAS} classes. Due to that:
- Symbol-component links are broken (e.g. component with LIB_ID
'Test/Name' will change to 'Test_Name', but the corresponding LIB_PART
will remain 'Test/Name'.
- It is impossible to place/edit such symbols.
The simplest solution is to process LIB_{PART,ALIAS} LIB_IDs in the same
way as id done for SCH_COMPONENTs, so they become valid names that match
SCH_COMPONENTs using them (see the attached patches). There are two
drawbacks:
- Changing names that user had previously assigned, which might be
annoying if LIB_IDs are used e.g. to create BOMs. Personally I would use
the value field that accepts all characters for this purpose.
- Naming conflicts may occur (e.g. both 'Test/Name' and 'Test:Name' will
be changed to 'Test_Name', but I believe it is a rare case.
Any thoughts? I believe a more elegant solution will be implemented once
the new schematic file format is ready.
Cheers,
Orson
1. https://bugs.launchpad.net/kicad/+bug/1752419
From b3061c2f8c4ec2723b93e58c7a6c0d09ef15298b Mon Sep 17 00:00:00 2001
From: Maciej Suminski
Date: Tue, 6 Mar 2018 10:11:54 +0100
Subject: [PATCH 1/4] Added ReplaceIllegalFileNameChars() for wxString&
---
common/string.cpp | 37 ++---
include/kicad_string.h | 1 +
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/common/string.cpp b/common/string.cpp
index 8f35b83d6..8932fbc18 100644
--- a/common/string.cpp
+++ b/common/string.cpp
@@ -482,8 +482,9 @@ wxString GetIllegalFileNameWxChars()
bool ReplaceIllegalFileNameChars( std::string* aName, int aReplaceChar )
{
-bool changed = false;
-std::string result;
+bool changed = false;
+std::string result;
+result.reserve( aName->length() );
for( std::string::iterator it = aName->begin(); it != aName->end(); ++it )
{
@@ -503,7 +504,37 @@ bool ReplaceIllegalFileNameChars( std::string* aName, int aReplaceChar )
}
if( changed )
-*aName = result;
+*aName = result;
+
+return changed;
+}
+
+
+bool ReplaceIllegalFileNameChars( wxString& aName, int aReplaceChar )
+{
+bool changed = false;
+wxString result;
+result.reserve( aName.Length() );
+
+for( wxString::iterator it = aName.begin(); it != aName.end(); ++it )
+{
+if( strchr( illegalFileNameChars, *it ) )
+{
+if( aReplaceChar )
+result += aReplaceChar;
+else
+result += wxString::Format( "%%%02x", *it );
+
+changed = true;
+}
+else
+{
+result += *it;
+}
+}
+
+if( changed )
+aName = result;
return changed;
}
diff --git a/include/kicad_string.h b/include/kicad_string.h
index abf6a8200..3a4f45fbe 100644
--- a/include/kicad_string.h
+++ b/include/kicad_string.h
@@ -172,6 +172,7 @@ wxString GetIllegalFileNameWxChars();
* @return true if any characters have been replaced in \a aName.
*/
bool ReplaceIllegalFileNameChars( std::string* aName, int aReplaceChar = 0 );
+bool ReplaceIllegalFileNameChars( wxString& aName, int aReplaceChar = 0 );
#ifndef HAVE_STRTOKR
// common/strtok_r.c optionally:
--
2.13.3
From f7a0ee82a6ea32d0c68c97c26079feed2ada61d3 Mon Sep 17 00:00:00 2001
From: Maciej Suminski
Date: Tue, 6 Mar 2018 11:09:11 +0100
Subject: [PATCH 2/4] Replace illegal characters in LIB_{ALIAS,PART} LIB_IDs
Schematic components have illegal characters replaced during load,
leading to broken component-symbol links. To avoid this, library symbols
should have their names fixed in the same way.
---
eeschema/class_libentry.cpp | 23 ---
eeschema/class_libentry.h | 2 +-
eeschema/lib_field.cpp | 16 ++--
3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/eeschema/class_libentry.cpp b/eeschema/class_libentry.cpp
index 4f7f8807b..09bab9467 100644
--- a/eeschema/class_libentry.cpp
+++ b/eeschema/class_libentry.cpp
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
@@ -67,7 +68,7 @@ LIB_ALIAS::LIB_ALIAS( const wxString& aName, LIB_PART* aRootPart ):
EDA_ITEM( LIB_ALIAS_T ),
shared( aRootPart )
{
-name = aName;
+SetName( aName );
}
@@ -118,6 +119,13 @@ PART_LIB* LIB_ALIAS::GetLib()
}
+void LIB_ALIAS::SetName( const wxString& aName )
+{
+name = aName;
+ReplaceIllegalFileNameChars( name, '_' );
+}
+
+
bool LIB_ALIAS::operator==( const wxChar* aName ) const
{
return name == aName;
@@ -275,21
