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
Optimize RAR's GetReferenceItems method #5929
Changes from 1 commit
cfe147c
17015af
fec708e
ed2bc5c
2fc517f
1dc3bc5
77b1038
507214d
4945ad2
cf314ab
7077b0e
afe2cf0
9dccb26
147ef43
2901929
fd0a93d
74cf67c
8a45a46
2a8893c
04e72ed
2bb3210
ff501aa
2f23808
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2598,9 +2598,6 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
// Set the CopyLocal metadata. | ||
referenceItem.SetMetadata(ItemMetadataNames.copyLocal, reference.IsCopyLocal.ToString()); | ||
|
||
// Set the FusionName metadata. | ||
referenceItem.SetMetadata(ItemMetadataNames.fusionName, fusionName); | ||
|
||
// Set the Redist name metadata. | ||
if (!String.IsNullOrEmpty(reference.RedistName)) | ||
{ | ||
|
@@ -2654,20 +2651,11 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
} | ||
} | ||
|
||
// Set the IsRedistRoot metadata | ||
if (reference.IsRedistRoot == true) | ||
{ | ||
referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, "true"); | ||
} | ||
else if (reference.IsRedistRoot == false) | ||
{ | ||
referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, "false"); | ||
} | ||
else | ||
// The redist root is "null" when there was no IsRedistRoot flag in the Redist XML | ||
// (or there was no redist XML at all for this item). | ||
if (reference.IsRedistRoot != null) | ||
{ | ||
// This happens when the redist root is "null". This means there | ||
// was no IsRedistRoot flag in the Redist XML (or there was no | ||
// redist XML at all for this item). | ||
referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, reference.IsRedistRoot.ToString()); | ||
} | ||
|
||
// If there was a primary source item, then forward metadata from it. | ||
|
@@ -2726,14 +2714,16 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
referenceItem.SetMetadata(ItemMetadataNames.version, String.Empty); | ||
} | ||
|
||
// Unset fusionName so we don't have to unset it later. | ||
referenceItem.RemoveMetadata(ItemMetadataNames.fusionName); | ||
|
||
// Now clone all properties onto the related files. | ||
foreach (string relatedFileExtension in reference.GetRelatedFileExtensions()) | ||
{ | ||
ITaskItem item = new TaskItem(reference.FullPathWithoutExtension + relatedFileExtension); | ||
// Clone metadata. | ||
referenceItem.CopyMetadataTo(item); | ||
// Related files don't have a fusion name. | ||
item.SetMetadata(ItemMetadataNames.fusionName, ""); | ||
RemoveNonForwardableMetadata(item); | ||
|
||
// Add the related item. | ||
|
@@ -2749,7 +2739,6 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
// Set the destination directory. | ||
item.SetMetadata(ItemMetadataNames.destinationSubDirectory, FileUtilities.EnsureTrailingSlash(Path.GetDirectoryName(satelliteFile))); | ||
// Satellite files don't have a fusion name. | ||
item.SetMetadata(ItemMetadataNames.fusionName, ""); | ||
RemoveNonForwardableMetadata(item); | ||
|
||
// Add the satellite item. | ||
|
@@ -2763,7 +2752,6 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
// Clone metadata. | ||
referenceItem.CopyMetadataTo(item); | ||
// serialization assemblies files don't have a fusion name. | ||
item.SetMetadata(ItemMetadataNames.fusionName, ""); | ||
RemoveNonForwardableMetadata(item); | ||
|
||
// Add the serialization assembly item. | ||
|
@@ -2777,7 +2765,6 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
// Clone metadata. | ||
referenceItem.CopyMetadataTo(item); | ||
// We don't have a fusion name for scatter files. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove these comments please. |
||
item.SetMetadata(ItemMetadataNames.fusionName, ""); | ||
RemoveNonForwardableMetadata(item); | ||
|
||
// Add the satellite item. | ||
|
@@ -2798,6 +2785,9 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> | |
} | ||
} | ||
|
||
// Set the FusionName metadata properly. | ||
referenceItem.SetMetadata(ItemMetadataNames.fusionName, fusionName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does "properly" mean? In general, how do you know this is safe? Is the local variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If any of reference.GetSourceItems() has a fusionName, or reference.PrimarySourceItem has one, fusionName may have been set to an invalid value at this point. We remove it before forwarding referenceItem's metadata to other items, but we still need to set it properly at the end, i.e., give it the right fusionName. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you mind clarifying what you meant about fusionName being guaranteed to be empty? Previously, fusionName was set to a value near the top of this method, that value was passed on to a variety of "relatedItems," "satelliteItems," etc., and then it was unset from all of those other items. Here, instead, I'm unsetting it prior to using fusionName for any of those, and only giving it its value at the bottom, so CopyMetadataTo automatically skips fusionName. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The meta comment is: "properly" is basically useless in a code comment. Comments describe what you want the code to do/why it's done, and of course you want to do things "properly". Instead, describe details. Here, you might have a comment that |
||
|
||
return referenceItem; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert to ternary operator