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

Enable ImplictUsings and use file-scoped namespaces #693

Merged
merged 8 commits into from Jan 24, 2023
Merged

Enable ImplictUsings and use file-scoped namespaces #693

merged 8 commits into from Jan 24, 2023

Conversation

iamcarbon
Copy link
Sponsor Contributor

This PR is up for feedback on whether we should enable file-scoped namespaces on some of all of the markdig projects.

Pulling the indentation in 4 lines on the tests is a fairly nice improvement for readability -- particularly on a 15" laptop.

@iamcarbon
Copy link
Sponsor Contributor Author

@MihaZupan @xoofx Let me know what you think. If we do make this change on the main library -- it would make the change history harder to follow -- but improve the experience working with those files moving forward.

@coveralls
Copy link

coveralls commented Jan 19, 2023

Pull Request Test Coverage Report for Build 3977316982

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 17977 of 18888 (95.18%) changed or added relevant lines in 306 files are covered.
  • 246 unchanged lines in 72 files lost coverage.
  • Overall coverage remained the same at 93.292%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Markdig.Tests/Program.cs 0 1 0.0%
src/Markdig.Tests/TestMediaLinks.cs 40 41 97.56%
src/Markdig.Tests/TestYamlFrontMatterExtension.cs 37 38 97.37%
src/Markdig/Extensions/Figures/FigureBlockParser.cs 49 50 98.0%
src/Markdig/Helpers/FastStringWriter.cs 142 143 99.3%
src/Markdig/Helpers/LineReader.cs 31 32 96.88%
src/Markdig/Helpers/StringBuilderExtensions.cs 0 1 0.0%
src/Markdig/MarkdownParserContext.cs 0 1 0.0%
src/Markdig/Parsers/HeadingBlockParser.cs 81 82 98.78%
src/Markdig/Parsers/HtmlBlockParser.cs 235 236 99.58%
Files with Coverage Reduction New Missed Lines %
src/Markdig/Extensions/Abbreviations/AbbreviationParser.cs 1 95.6%
src/Markdig/Extensions/Emoji/EmojiInline.cs 1 57.14%
src/Markdig/Extensions/Figures/FigureBlockParser.cs 1 95.7%
src/Markdig/Extensions/Footnotes/FootnoteParser.cs 1 93.07%
src/Markdig/Extensions/GenericAttributes/GenericAttributesExtension.cs 1 96.77%
src/Markdig/Extensions/Globalization/GlobalizationExtension.cs 1 86.88%
src/Markdig/Extensions/Mathematics/MathBlockParser.cs 1 92.11%
src/Markdig/Extensions/MediaLinks/MediaLinkExtension.cs 1 93.02%
src/Markdig/Extensions/TaskLists/HtmlTaskListRenderer.cs 1 60.71%
src/Markdig/Extensions/Yaml/YamlFrontMatterParser.cs 1 91.67%
Totals Coverage Status
Change from base Build 3919526952: 0.0%
Covered Lines: 25916
Relevant Lines: 27171

💛 - Coveralls

@xoofx
Copy link
Owner

xoofx commented Jan 21, 2023

@MihaZupan @xoofx Let me know what you think. If we do make this change on the main library -- it would make the change history harder to follow -- but improve the experience working with those files moving forward.

Yeah, please go ahead in applying this to the rest of the library, it should be ok. Thanks!

@MihaZupan
Copy link
Collaborator

I like the feature, thanks!
My only concern is the quality of the automatic refactoring - when we did this in YARP it messed up indentation in random places. We should just double-check that doesn't happen before merging.

@iamcarbon
Copy link
Sponsor Contributor Author

@MihaZupan Agreed on the caution -- I used the refactoring tools to do this file by file, checking them as I went along to make sure everything was indented correctly. We should be OK!

@iamcarbon
Copy link
Sponsor Contributor Author

@xoofx @MihaZupan Ready for review. I also fixed a couple spelling errors in the comments while I was double checking the indentation levels.

@iamcarbon
Copy link
Sponsor Contributor Author

Here's a direct link to review ignoring whitespace changes: https://github.com/xoofx/markdig/pull/693/files?diff=unified&w=1

@xoofx xoofx merged commit 851713f into xoofx:master Jan 24, 2023
@xoofx
Copy link
Owner

xoofx commented Jan 24, 2023

Thanks a lot!

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.

None yet

4 participants