Skip to content
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

Merged
merged 38 commits into from
Oct 28, 2020
Merged

Xml to markdown with Workflow #132

merged 38 commits into from
Oct 28, 2020

Conversation

twofingerrightclick
Copy link
Collaborator

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.

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
@twofingerrightclick twofingerrightclick changed the title Xml to markdown with action Xml to markdown with Workflow Oct 23, 2020

using System.Diagnostics.CodeAnalysis;

[assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "<Pending>")]
Copy link
Member

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>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
public List<string> Comments { get;} = new List<string>();
public List<string> Comments { get; } = new List<string>();

{
static class GuidelineXmlFileReader
{
public const string _Guideline = "guideline";
Copy link
Member

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 {
Copy link
Member

@Keboo Keboo Oct 28, 2020

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>")]
Copy link
Member

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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")]
Copy link
Member

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);
Copy link
Member

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.

Suggested change
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) {
Copy link
Member

@Keboo Keboo Oct 28, 2020

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
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 {
Copy link
Member

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");
Copy link
Member

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
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)
Copy link
Member

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) {
Copy link
Member

@Keboo Keboo Oct 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
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));
Copy link
Member

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};
Copy link
Member

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.

Comment on lines +47 to +48
//C: \Users\saffron\source\repos\CodingGuidelines\XMLtoMD\GuidelineXmlToMD\bin\Debug\netcoreapp3.1
//C: \Users\saffron\source\repos\CodingGuidelines\docs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//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)
Copy link
Member

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)
Copy link
Member

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.

Comment on lines +128 to +130
subSections = (from guideline in guidelines
where string.Equals(guideline.Section, section, System.StringComparison.OrdinalIgnoreCase)
select guideline.Subsection).Distinct().OrderBy(x => x).ToList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);


}


Copy link
Member

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.

@Keboo Keboo merged commit 9fab791 into master Oct 28, 2020
@Keboo Keboo deleted the XmlToMarkdown branch October 28, 2020 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants