-
Notifications
You must be signed in to change notification settings - Fork 116
Refactor formatter config to reqnroll json config #1024
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
Draft
clrudolphi
wants to merge
6
commits into
main
Choose a base branch
from
Refactor_Formatter_Config_to_Reqnroll_JsonConfig
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
bad7b52
Added Formatters element to the JsonConfig structure. Using JsonConfi…
clrudolphi a55235a
Modified the primary interface contract for the Formatters configurat…
clrudolphi 2447b42
Update CHANGELOG.md
clrudolphi cc9f4f2
Merge branch 'main' into Refactor_Formatter_Config_to_Reqnroll_JsonCo…
clrudolphi 5491922
Modified per review comments. Removed old property rather than mark a…
clrudolphi 5735eae
Fully integrated Formatter configuration with Reqnroll Configuration …
clrudolphi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 18 additions & 0 deletions
18
Reqnroll/Configuration/JsonConfig/FormatterOptionsElement.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| using System.Collections.Generic; | ||
| using System.Text.Json; | ||
| using System.Text.Json.Serialization; | ||
|
|
||
| namespace Reqnroll.Configuration.JsonConfig | ||
| { | ||
| public class FormatterOptionsElement | ||
| { | ||
| [JsonPropertyName("outputFilePath")] | ||
| public string OutputFilePath { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Captures any additional options not explicitly defined above. | ||
| /// </summary> | ||
| [JsonExtensionData] | ||
| public Dictionary<string, JsonElement> AdditionalOptions { get; set; } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| using System.Collections.Generic; | ||
| using System.Text.Json; | ||
| using System.Text.Json.Serialization; | ||
|
|
||
| namespace Reqnroll.Configuration.JsonConfig | ||
| { | ||
| public class FormattersElement | ||
| { | ||
| [JsonPropertyName("html")] | ||
| public FormatterOptionsElement Html { get; set; } | ||
|
|
||
| [JsonPropertyName("message")] | ||
| public FormatterOptionsElement Message { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Captures any additional/custom formatters not explicitly defined above. | ||
| /// </summary> | ||
| [JsonExtensionData] | ||
| public Dictionary<string, JsonElement> AdditionalFormatters { get; set; } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
133 changes: 133 additions & 0 deletions
133
Reqnroll/Formatters/Configuration/FormatterConfiguration.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| #nullable enable | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace Reqnroll.Formatters.Configuration; | ||
|
|
||
| /// <summary> | ||
| /// Represents the configuration for a formatter with type-safe access to known properties | ||
| /// and extensibility for custom settings. | ||
| /// </summary> | ||
| public class FormatterConfiguration | ||
| { | ||
| /// <summary> | ||
| /// The output file path for the formatter. May be null if not configured. | ||
| /// </summary> | ||
| public string? OutputFilePath { get; set; } | ||
|
gasparnagy marked this conversation as resolved.
|
||
|
|
||
| /// <summary> | ||
| /// Additional settings for the formatter that are not explicitly defined as properties. | ||
| /// </summary> | ||
| public IDictionary<string, object> AdditionalSettings { get; set; } = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| /// <summary> | ||
| /// Creates a FormatterConfiguration from a dictionary representation. | ||
| /// </summary> | ||
| /// <param name="dictionary">The dictionary containing formatter configuration values.</param> | ||
| /// <returns>A new FormatterConfiguration instance, or null if the dictionary is null.</returns> | ||
| public static FormatterConfiguration? FromDictionary(IDictionary<string, object>? dictionary) | ||
| { | ||
| if (dictionary == null) | ||
| return null; | ||
|
|
||
| var config = new FormatterConfiguration(); | ||
| var additionalSettings = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| foreach (var kvp in dictionary) | ||
| { | ||
| if (string.Equals(kvp.Key, "outputFilePath", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| config.OutputFilePath = kvp.Value?.ToString(); | ||
| } | ||
| else if (kvp.Value != null) | ||
| { | ||
| additionalSettings[kvp.Key] = kvp.Value; | ||
| } | ||
| } | ||
|
|
||
| config.AdditionalSettings = additionalSettings; | ||
| return config; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Converts this FormatterConfiguration back to a dictionary representation. | ||
| /// Used for backward compatibility with legacy APIs. | ||
| /// </summary> | ||
| /// <returns>A dictionary containing all configuration values.</returns> | ||
| public IDictionary<string, object> ToDictionary() | ||
| { | ||
| var result = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| if (OutputFilePath != null) | ||
| { | ||
| result["outputFilePath"] = OutputFilePath; | ||
| } | ||
|
|
||
| foreach (var kvp in AdditionalSettings) | ||
| { | ||
| result[kvp.Key] = kvp.Value; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a configuration value by key, checking both known properties and additional settings. | ||
| /// </summary> | ||
| /// <typeparam name="T">The expected type of the value.</typeparam> | ||
| /// <param name="key">The configuration key.</param> | ||
| /// <param name="defaultValue">The default value if the key is not found.</param> | ||
| /// <returns>The configuration value or the default value.</returns> | ||
| public T? GetValue<T>(string key, T? defaultValue = default) | ||
| { | ||
| if (string.Equals(key, "outputFilePath", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| if (OutputFilePath is T typedValue) | ||
| return typedValue; | ||
| return defaultValue; | ||
| } | ||
|
|
||
| if (AdditionalSettings.TryGetValue(key, out var value)) | ||
| { | ||
| if (value is T typedValue) | ||
| return typedValue; | ||
|
|
||
| // Try conversion for common types | ||
| try | ||
| { | ||
| return (T)Convert.ChangeType(value, typeof(T)); | ||
| } | ||
| catch | ||
| { | ||
| return defaultValue; | ||
| } | ||
| } | ||
|
|
||
| return defaultValue; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Merges settings from another FormatterConfiguration into this one. | ||
| /// Only non-null values from the other configuration will override values in this configuration. | ||
| /// This allows partial overrides where only specified settings are changed. | ||
| /// </summary> | ||
| /// <param name="other">The configuration to merge from. Null values are ignored.</param> | ||
| public void MergeFrom(FormatterConfiguration? other) | ||
| { | ||
| if (other == null) | ||
| return; | ||
|
|
||
| // Only override OutputFilePath if the other configuration has it set | ||
| if (other.OutputFilePath != null) | ||
| { | ||
| OutputFilePath = other.OutputFilePath; | ||
| } | ||
|
|
||
| // Merge additional settings - other's values override this's values | ||
| foreach (var kvp in other.AdditionalSettings) | ||
| { | ||
| AdditionalSettings[kvp.Key] = kvp.Value; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is this not a
Dictionary<string,FormatterOptionsElement>?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.
I haven't found a way to do that directly. The Json source generator limits extension data to JsonElements, JsonNodes, or objects.
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.
But we could make the whole thing as a
Dictionary<string,FormatterOptionsElement>, right? That would implicitly contain html and message as keys, but we could make prop getters to access them easily.