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

Add a helper function to add user defined properties to the user defined properties container. #190

Merged
merged 3 commits into from
Oct 15, 2024

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Oct 12, 2024

Refs comments in #104 - I think it'd be useful to have a built in function that handles the management of property identifiers and names so that users don't have to do it manually.

Question: The names of user defined properties are required to be unique within a file so it needs some validation.
Should an attempt to add a user defined property with a name that already exists be an error (e.g. throw an exception), or should it replace any existing property with a new one (it might have to handle the new property having a different type than the existing one, which would need to remove the existing property and create a new one)

/// <returns>The new property, of null if this container can't contain user defined properties.</returns>
public OLEProperty AddUserDefinedProperty(VTPropertyType vtPropertyType, string name)
{
if (this.ContainerType != ContainerType.UserDefinedProperties)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it could be done so that calls against a DocumentSummaryInfo container forward on to the contained UserDefinedProperties container. (Now that there is a CreateUserDefinedProperties() function, it could even create the UserDefinedProperties container if there isn't one already)

@ironfede
Copy link
Owner

Refs comments in #104 - I think it'd be useful to have a built in function that handles the management of property identifiers and names so that users don't have to do it manually.

Question: The names of user defined properties are required to be unique within a file so it needs some validation. Should an attempt to add a user defined property with a name that already exists be an error (e.g. throw an exception), or should it replace any existing property with a new one (it might have to handle the new property having a different type than the existing one, which would need to remove the existing property and create a new one)

I think that an exception is more linear and makes things easier to spot if this behaviour appears as a wrong action (>90% cases I suppose)

@Numpsy Numpsy force-pushed the add_user_defined_property_2 branch from d0c819d to 0135b53 Compare October 13, 2024 09:04

// As per https://learn.microsoft.com/en-us/openspecs/windows_protocols/MS-OLEPS/4177a4bc-5547-49fe-a4d9-4767350fd9cf
// the property names have to be unique, and are case insensitive.
if (this.PropertyNames.Any(property => property.Value.Equals(name, StringComparison.InvariantCultureIgnoreCase)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually sure if the uniqueness of the user defined property names needs to consider locales / cultures here?

@Numpsy Numpsy force-pushed the add_user_defined_property_2 branch from a0c22f9 to 0cdb887 Compare October 14, 2024 22:23
This was referenced Oct 14, 2024
@Numpsy Numpsy marked this pull request as ready for review October 15, 2024 13:18
@ironfede ironfede merged commit d394694 into ironfede:master Oct 15, 2024
1 check passed
@Numpsy Numpsy deleted the add_user_defined_property_2 branch October 20, 2024 09:55
@Numpsy Numpsy mentioned this pull request Nov 14, 2024
jeremy-visionaid added a commit that referenced this pull request Nov 15, 2024
jeremy-visionaid added a commit that referenced this pull request Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants