-
Notifications
You must be signed in to change notification settings - Fork 17
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
Xml to markdown with Workflow #132
Conversation
add Guideline xml to md project
The markdown out proj was framework 4.5
Moved MarkdownOut files to the XML project
use system specific set path directory seperator for file access
Co-authored-by: Kevin B <Keboo@users.noreply.github.com>
The null public parameter warnings were apart of code copied from an old project. This code isn't going to be used in production, so I didn't refactor.
Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Trying to figure out how to have a string env variable for the job Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml Update ConvertGuidelinesXmlToMarkdown.yml positive logic git hub actions proceed only if the xml file was modified in the previous commit ( git diff HEAD~1 HEAD) get xml filename as argument Update ConvertGuidelinesXmlToMarkdown.yml
|
||
using System.Diagnostics.CodeAnalysis; | ||
|
||
[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "<Pending>")] |
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.
Suppressed warnings should have Justification filled out or be addressed.
public string Section { get; set; } = ""; | ||
public string Subsection { get; set; } = ""; | ||
|
||
public List<string> Comments { get;} = new List<string>(); |
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.
nit:
public List<string> Comments { get;} = new List<string>(); | |
public List<string> Comments { get; } = new List<string>(); |
{ | ||
static class GuidelineXmlFileReader | ||
{ | ||
public const string _Guideline = "guideline"; |
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.
Would be nice if these constants were put in their own static class. Since these are related to the structure of the XML and should be put in a class with a name that reflects that.
An even cleaner option is to use xsd
to generate the XML classes/serializer so that you don't need to do it yourself.
You can find the doc on that here
@@ -0,0 +1,47 @@ | |||
namespace MarkdownOut { |
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.
Braces should be put on their own line. The .editorconfig should be flagging these. If it isn't it could be because it is a level higher than the solution. If that is the case, don't have a separate solution for this project and just added to the main solution.
/// The selectively styled string. If <paramref name="str"/> does not contain any | ||
/// occurrences of <paramref name="substring"/>, it is returned unchanged. | ||
/// </returns> | ||
[System.Diagnostics.CodeAnalysis.SuppressMessage("Globalization", "CA1303:Do not pass literals as localized parameters", Justification = "<Pending>")] |
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.
This appears to duplicate the one in the global suppression file.
} | ||
|
||
if (!firstOnly) { | ||
return str?.Replace(substring, MdText.Style(substring, style), System.StringComparison.Ordinal); |
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.
return str?.Replace(substring, MdText.Style(substring, style), System.StringComparison.Ordinal); | |
return str.Replace(substring, MdText.Style(substring, style), System.StringComparison.Ordinal); |
/// <summary> | ||
/// Provides static fields and methods to style and format Markdown text. | ||
/// </summary> | ||
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "INTL0001:Fields _PascalCase", Justification = "From an Old Github Project, not for production")] |
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.
This should not need to be suppressed once the fields are removed (see below)
/// <summary> | ||
/// The Markdown tab string (four spaces). | ||
/// </summary> | ||
public static readonly string Tab = new string(' ', 4); |
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.
It would be cleaner to make all of these constants rather that static readonly.
public static readonly string Tab = new string(' ', 4); | |
public const string Tab = " "; |
/// <param name="style">The Markdown style to apply.</param> | ||
/// <returns>The styled string.</returns> | ||
/// <exception cref="System.ArgumentException">The style is not recognized.</exception> | ||
public static string Style(object text, MdStyle style) { |
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.
Can text
be converted to be a string
. There is an implicit ToString()
call that is occurring with the concatenation at the end. I would expect the type change to propagate up to the other methods in this file.
/// </param> | ||
/// <returns>The indented string.</returns> | ||
public static string Indent(object text, int indent) { | ||
string indentPrefix = String.Empty; |
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.
nit:
string indentPrefix = String.Empty; | |
string indentPrefix = string.Empty; |
/// Provides static fields and methods to style and format Markdown text. | ||
/// </summary> | ||
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "INTL0001:Fields _PascalCase", Justification = "From an Old Github Project, not for production")] | ||
public static class MdText { |
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.
Lots of string concatenation going on here. Often this can be improved by using a StringBuilder.
* done to make sure the raw text displays correctly on Windows machines (programs such | ||
* as Visual Studio or Notepad do not recognize "\n" as a newline, just "\r\n"). | ||
*/ | ||
text = text.Replace("\r\n", "\n").Replace("\r", "\n"); |
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.
Is this needed? When checking out files with git it will attempt to handle line endings for you based on your operating system.
/// <param name="format">The optional Markdown format to apply.</param> | ||
/// </param> | ||
public void WriteUnorderedListItem(object output, int listIndent = 0, | ||
MdStyle style = MdStyle.None, MdFormat format = MdFormat.None, int numNewLines=1) { |
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.
nit:
MdStyle style = MdStyle.None, MdFormat format = MdFormat.None, int numNewLines=1) { | |
MdStyle style = MdStyle.None, MdFormat format = MdFormat.None, int numNewLines = 1) { |
class Program | ||
{ | ||
static MarkdownOut.MdWriter _MdWriter; | ||
static void Main(string[] args) |
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.
You might consider leveraging System.CommandLine (specifically DragonFruit) to make this easier.
{ | ||
string xmlFileName = "Guidelines(8th Edition).xml"; | ||
if (args.Length != 0) { //check for input fileName | ||
if (Regex.Match(args[0], @".*.xml").Success) { |
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.
if (Regex.Match(args[0], @".*.xml").Success) { | |
if (Regex.IsMatch(args[0], @".*\.xml")) { |
Another alternative is to use Path.GetExtension
} | ||
} | ||
|
||
Match repoRefFolder = Regex.Match(AssemblyDirectory, @$".*CodingGuidelines"); |
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.
nit:
Match repoRefFolder = Regex.Match(AssemblyDirectory, @$".*CodingGuidelines"); | |
Match repoRefFolder = Regex.Match(AssemblyDirectory, $".*CodingGuidelines"); |
Match repoRefFolder = Regex.Match(AssemblyDirectory, @$".*CodingGuidelines"); | ||
|
||
string[] xmlFilePath = { repoRefFolder.Value, "docs", xmlFileName}; | ||
ICollection<Guideline> guidelines = GuidelineXmlFileReader.ReadExisitingGuidelinesFile(Path.Combine(xmlFilePath)); |
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.
It may be cleaner to simply pass in the path to the xml file as a required argument to Main rather than trying to derive it.
|
||
|
||
string mdFileName = "csharp.md"; | ||
string[] mdFilePath = { repoRefFolder.Value, "docs", "coding", mdFileName}; |
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.
This is another opportunity to pass in the path to the markdown file rather than deriving it.
//C: \Users\saffron\source\repos\CodingGuidelines\XMLtoMD\GuidelineXmlToMD\bin\Debug\netcoreapp3.1 | ||
//C: \Users\saffron\source\repos\CodingGuidelines\docs |
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.
//C: \Users\saffron\source\repos\CodingGuidelines\XMLtoMD\GuidelineXmlToMD\bin\Debug\netcoreapp3.1 | |
//C: \Users\saffron\source\repos\CodingGuidelines\docs |
Console.WriteLine($" { subsection}"); | ||
_MdWriter.WriteLine(subsection, format: MdFormat.Heading3, numNewLines: 1); | ||
|
||
foreach (Guideline guideline in guidelinesInSubsection) |
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.
This is trigging multiple enumerations of guidelinesInSubsection
. In these situations it might be simpler to change the type to IList<Guideline>
and call ToList
on the end of the LINQ expression.
|
||
} | ||
|
||
public static List<string> GetSections(ICollection<Guideline> guidelines) |
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.
In general it is better to make methods like this be IEnumerable
and if the callers want a list they can call ToList
on the return value.
subSections = (from guideline in guidelines | ||
where string.Equals(guideline.Section, section, System.StringComparison.OrdinalIgnoreCase) | ||
select guideline.Subsection).Distinct().OrderBy(x => x).ToList(); |
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.
subSections = (from guideline in guidelines | |
where string.Equals(guideline.Section, section, System.StringComparison.OrdinalIgnoreCase) | |
select guideline.Subsection).Distinct().OrderBy(x => x).ToList(); | |
IEnumerable<string> subSections = (from guideline in guidelines | |
where string.Equals(guideline.Section, section, System.StringComparison.OrdinalIgnoreCase) | |
select guideline.Subsection).Distinct().OrderBy(x => x); |
|
||
} | ||
|
||
|
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.
Lots of blank lines that can be cleaned up here.
If the master branch has a change to its xml guideline file then there is a github workflow that will reproduce the csharp.md file. The name of the xml guideline file is a Environment variable in the github workflow yml. Currently its
Guidelines(8th Edition).xml
.