-
Notifications
You must be signed in to change notification settings - Fork 55
Register new display encoders when loading packages. #179
Conversation
rmshaffer
left a comment
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.
Looks nice, thanks for doing this! Just a couple of small comments.
src/Kernel/IQSharpEngine.cs
Outdated
| .Select(asm => asm.Assembly.GetName()) | ||
| .ToImmutableHashSet() | ||
| // Except assemblies known at compile-time as well. | ||
| .Add(typeof(StateVectorToHtmlResultEncoder).Assembly.GetName()); |
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.
When #177 is merged, the AzureClient assembly will also have some display encoders in it. Just FYI to whichever one of us merges second.
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.
Good to know, thanks! Given what you said about conda breaking until #177, it probably makes sense for that one to go first and for me to catch up?
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.
Probably yes, that makes sense. Hoping to be able to merge that first thing tomorrow morning.
This PR adds a new event handler that loads display encoders from new packages as they are loaded, allowing packages to extend the display encoder mechanism to handle types not known at IQ# compile time.