Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle cases where legal IP-XACT names are not legal SystemRDL names #8

Closed
jasonpjacobs opened this issue Jan 12, 2022 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@jasonpjacobs
Copy link

The IP-XACT spec (1685-2014, Section D.7) has the following description of allowed characters in names:

The Name type defines a series of any characters, excluding embedded whitespace. It needs to begin with a
letter, colon (:), or underscore (). A Name shall contain only letters, numbers, and the colon (:),
underscore (), dash (-), and dot (.) characters. Any leading or trailing spaces are removed.
This seems like a straightforward fix that I may take on if it isn't already planned.

However, SystemRDL's rules about identifiers are more strict:

An identifier assigns a name to a user-defined data type or its instance. There are two types of identifiers:
simple and escaped. Identifiers are case-sensitive. Simple identifiers have a first character that is a letter or
underscore (_) followed by zero or more letters, digits, and underscores. Escaped identifiers begin with
followed by a simple identifier.

Note, escaped identifiers allow one to use a SystemRDL keyword as an identifier, but don't allow the use of dots, dashes, colons, etc.

I'm happy to author a pull request to implement this. Writing a function/method to translate an IP-XACT name to a valid SystemRDL one is straightforward. Where might be the best place to introduce this feature? Users will likely want to control whether the transformation is performed as well as the approach taken. I think either a class method that can be overridden, or a stand alone function that is passed into importer's initializer. Thoughts?

@amykyta3 amykyta3 added the bug Something isn't working label Jan 14, 2022
@amykyta3
Copy link
Member

Fixed and published v2.1.2

@jasonpjacobs
Copy link
Author

jasonpjacobs commented Jan 14, 2022

Wow.. thanks for the quick response and commit! Can we expose an initializer argument that controls this feature? There are cases where I'll want to perform the name sanitization, and other cases were importing incompatible names should raise an exception.

@amykyta3
Copy link
Member

amykyta3 commented Jan 15, 2022 via email

@jasonpjacobs
Copy link
Author

How about giving users the ability to provide a callable to use? The library itself can provide a function similar to what you've done already. But if for example I wanted to keep track of the names changes that were made I could create a class with a call method that returns the sanitized name, and in the implementation store a mapping of the original name to the mapped name.

@amykyta3
Copy link
Member

Recommend simply extending the exporter class and enhancing the function itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants