-
Notifications
You must be signed in to change notification settings - Fork 10
Fix csharp_style_namespace_declarations EditorConfig support for step definition generation #106
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
base: main
Are you sure you want to change the base?
Changes from all commits
dc21623
f5b0c53
61bebbc
1004f48
ee82784
ad7fb00
655c69b
31235ce
3ac3a40
8d61bc8
d4010d2
b84db03
3b6606e
9d0ae71
ce41e66
36f5be8
2c49097
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 |
---|---|---|
@@ -0,0 +1,18 @@ | ||
namespace Reqnroll.VisualStudio.Configuration; | ||
|
||
public class CSharpCodeGenerationConfiguration | ||
{ | ||
/// <summary> | ||
/// Specifies the namespace declaration style for generated C# code. | ||
/// Uses file-scoped namespaces when set to "file_scoped", otherwise uses block-scoped namespaces. | ||
/// </summary> | ||
[EditorConfigSetting("csharp_style_namespace_declarations")] | ||
public string NamespaceDeclarationStyle { get; set; } = "block_scoped"; | ||
|
||
/// <summary> | ||
/// Determines if file-scoped namespaces should be used based on the EditorConfig setting. | ||
/// </summary> | ||
public bool UseFileScopedNamespaces => | ||
NamespaceDeclarationStyle != null && | ||
NamespaceDeclarationStyle.StartsWith("file_scoped", StringComparison.OrdinalIgnoreCase); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,23 @@ | ||
#nullable disable | ||
|
||
using System.Text; | ||
|
||
namespace Reqnroll.VisualStudio.Editor.Commands; | ||
|
||
[Export(typeof(IDeveroomFeatureEditorCommand))] | ||
public class DefineStepsCommand : DeveroomEditorCommandBase, IDeveroomFeatureEditorCommand | ||
{ | ||
private readonly IEditorConfigOptionsProvider _editorConfigOptionsProvider; | ||
|
||
[ImportingConstructor] | ||
public DefineStepsCommand( | ||
IIdeScope ideScope, | ||
IBufferTagAggregatorFactoryService aggregatorFactory, | ||
IDeveroomTaggerProvider taggerProvider) | ||
IDeveroomTaggerProvider taggerProvider, | ||
IEditorConfigOptionsProvider editorConfigOptionsProvider) | ||
: base(ideScope, aggregatorFactory, taggerProvider) | ||
{ | ||
_editorConfigOptionsProvider = editorConfigOptionsProvider; | ||
} | ||
|
||
public override DeveroomEditorCommandTargetKey[] Targets => new[] | ||
|
@@ -101,7 +108,7 @@ public override bool PreExec(IWpfTextView textView, DeveroomEditorCommandTargetK | |
switch (viewModel.Result) | ||
{ | ||
case CreateStepDefinitionsDialogResult.Create: | ||
SaveAsStepDefinitionClass(projectScope, combinedSnippet, viewModel.ClassName, indent, newLine); | ||
SaveAsStepDefinitionClass(projectScope, combinedSnippet, viewModel.ClassName, indent, newLine, textView); | ||
break; | ||
case CreateStepDefinitionsDialogResult.CopyToClipboard: | ||
Logger.LogVerbose($"Copy to clipboard: {combinedSnippet}"); | ||
|
@@ -114,7 +121,7 @@ public override bool PreExec(IWpfTextView textView, DeveroomEditorCommandTargetK | |
} | ||
|
||
private void SaveAsStepDefinitionClass(IProjectScope projectScope, string combinedSnippet, string className, | ||
string indent, string newLine) | ||
string indent, string newLine, IWpfTextView textView) | ||
{ | ||
string targetFolder = projectScope.ProjectFolder; | ||
var projectSettings = projectScope.GetProjectSettings(); | ||
|
@@ -130,29 +137,62 @@ private void SaveAsStepDefinitionClass(IProjectScope projectScope, string combin | |
var isSpecFlow = projectTraits.HasFlag(ReqnrollProjectTraits.LegacySpecFlow) || projectTraits.HasFlag(ReqnrollProjectTraits.SpecFlowCompatibility); | ||
var libraryNameSpace = isSpecFlow ? "SpecFlow" : "Reqnroll"; | ||
|
||
var template = "using System;" + newLine + | ||
$"using {libraryNameSpace};" + newLine + | ||
newLine + | ||
$"namespace {fileNamespace}" + newLine + | ||
"{" + newLine + | ||
$"{indent}[Binding]" + newLine + | ||
$"{indent}public class {className}" + newLine + | ||
$"{indent}{{" + newLine + | ||
combinedSnippet + | ||
$"{indent}}}" + newLine + | ||
"}" + newLine; | ||
// Get C# code generation configuration from EditorConfig using target .cs file path | ||
var targetFilePath = Path.Combine(targetFolder, className + ".cs"); | ||
var csharpConfig = new CSharpCodeGenerationConfiguration(); | ||
var editorConfigOptions = _editorConfigOptionsProvider.GetEditorConfigOptionsByPath(targetFilePath); | ||
editorConfigOptions.UpdateFromEditorConfig(csharpConfig); | ||
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. 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. Found and fixed the root cause! There was a critical bug in Fixed in commit 9d0ae71:
The fix ensures that |
||
|
||
// Estimate template size for StringBuilder capacity | ||
var estimatedSize = 200 + fileNamespace.Length + className.Length + combinedSnippet.Length; | ||
var template = new StringBuilder(estimatedSize); | ||
template.AppendLine("using System;"); | ||
template.AppendLine($"using {libraryNameSpace};"); | ||
template.AppendLine(); | ||
|
||
// Determine indentation level based on namespace style | ||
var classIndent = csharpConfig.UseFileScopedNamespaces ? "" : indent; | ||
|
||
// Adjust combinedSnippet indentation based on namespace style | ||
var adjustedSnippet = csharpConfig.UseFileScopedNamespaces | ||
? AdjustIndentationForFileScopedNamespace(combinedSnippet, indent, newLine) | ||
: combinedSnippet; | ||
// Add namespace declaration | ||
if (csharpConfig.UseFileScopedNamespaces) | ||
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. @copilot deduplicate this part, it's almost the same as the else 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. Deduplicated the template generation code by extracting the common parts (using statements) and only varying the namespace-specific sections. This eliminates the code duplication while maintaining the exact same functionality. Fixed in commit 655c69b. 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. @copilot this is still duplicated? 90% of the difference is the indent, Maybe convert it to a stringbuilder? So I mean the template += in the if else below 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. Fixed the code duplication by refactoring to use StringBuilder approach. The template generation now builds the namespace and class parts separately with appropriate indentation, eliminating 90% of the redundant code between the if-else branches. Fixed in commit d4e086a. |
||
{ | ||
template.AppendLine($"namespace {fileNamespace};"); | ||
template.AppendLine(); | ||
} | ||
else | ||
{ | ||
template.AppendLine($"namespace {fileNamespace}"); | ||
template.AppendLine("{"); | ||
} | ||
|
||
// Add class declaration (common structure with appropriate indentation) | ||
template.AppendLine($"{classIndent}[Binding]"); | ||
template.AppendLine($"{classIndent}public class {className}"); | ||
template.AppendLine($"{classIndent}{{"); | ||
template.Append(adjustedSnippet); | ||
template.AppendLine($"{classIndent}}}"); | ||
|
||
// Close namespace if block-scoped | ||
if (!csharpConfig.UseFileScopedNamespaces) | ||
{ | ||
template.AppendLine("}"); | ||
} | ||
|
||
var targetFile = FileDetails | ||
.FromPath(targetFolder, className + ".cs") | ||
.WithCSharpContent(template); | ||
.WithCSharpContent(template.ToString()); | ||
|
||
if (IdeScope.FileSystem.File.Exists(targetFile.FullName)) | ||
if (IdeScope.Actions.ShowSyncQuestion("Overwrite file?", | ||
$"The selected step definition file '{targetFile}' already exists. By overwriting the existing file you might lose work. {Environment.NewLine}Do you want to overwrite the file?", | ||
defaultButton: MessageBoxResult.No) != MessageBoxResult.Yes) | ||
return; | ||
|
||
projectScope.AddFile(targetFile, template); | ||
projectScope.AddFile(targetFile, template.ToString()); | ||
projectScope.IdeScope.Actions.NavigateTo(new SourceLocation(targetFile, 9, 1)); | ||
IDiscoveryService discoveryService = projectScope.GetDiscoveryService(); | ||
|
||
|
@@ -168,4 +208,31 @@ await discoveryService.BindingRegistryCache | |
|
||
Finished.Set(); | ||
} | ||
|
||
private static string AdjustIndentationForFileScopedNamespace(string snippet, string indent, string newLine) | ||
{ | ||
if (string.IsNullOrEmpty(snippet)) | ||
return snippet; | ||
|
||
// Split into lines and process each line | ||
var lines = snippet.Split(new[] { newLine }, StringSplitOptions.None); | ||
var adjustedLines = new string[lines.Length]; | ||
|
||
for (int i = 0; i < lines.Length; i++) | ||
{ | ||
var line = lines[i]; | ||
|
||
// If line starts with double indentation, reduce it to single indentation | ||
if (line.StartsWith(indent + indent)) | ||
{ | ||
adjustedLines[i] = indent + line.Substring((indent + indent).Length); | ||
} | ||
else | ||
{ | ||
adjustedLines[i] = line; | ||
} | ||
} | ||
|
||
return string.Join(newLine, adjustedLines); | ||
} | ||
} |
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.
@copilot
it only works with
and not with
Why is that? Please fix
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.
Fixed the root cause! Added GetEditorConfigOptionsByPath method to get EditorConfig settings for the target .cs file path instead of the source .feature file. Now [*.cs] sections in .editorconfig will work correctly.