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

Feature request: Additional option for the lib to work on a single file rather than source and generated file #273

Closed
mysticmind opened this issue Aug 14, 2020 · 10 comments
Labels
Milestone

Comments

@mysticmind
Copy link
Contributor

mysticmind commented Aug 14, 2020

Problem at hand

  • With the current functionality, it requires 2 files to be maintained i.e file.source.md with the snippet markers and the tool generates file.md with the snippet content.
  • For some use cases such as Github readme files or in general, it is a bit cumbersome and convoluted to maintain source and generated files in source code control. Also this requires clear documentation and instructions in projects for contributors to only edit the source file and not the generated one.

Proposed solution

  • Given that the source file has markers snippet: name and the generated file has content within <!-- snippet: name --> and <!-- endsnippet -->. It is quite easy to detect the snippet content block based on these 2 patterns without any ambiguity and replace with the generated snippet on the same file.

My request is to add an option for the tool to work on a single file based on these patterns.

@SimonCropp
Copy link
Owner

ok. i will have this done in a couple of days

@SimonCropp SimonCropp added this to the 20.0.0 milestone Aug 17, 2020
@SimonCropp
Copy link
Owner

hows this mysticmind/reversemarkdown-net#81 ?

@SimonCropp
Copy link
Owner

@mysticmind
Copy link
Contributor Author

Thanks @SimonCropp, this will be really helpful feature.

@claremacrae
Copy link
Contributor

claremacrae commented Aug 18, 2020

I've had a go at using on some of the files in ApprovalTests.cpp...

I'll update this as I go, but the pattern for updating seems to be:

Preparation

  • Make sure you have updated to the beta
    • dotnet tool update --version 20.0.0-beta.3 -g MarkdownSnippets.Tool
  • Remove WriteHeader line from your mdsnippets.json file
  • Rerun the tool and commit the .md files (lots of changes such as endtoc -> endToc)

Migration

  • Delete all your .source.md files
  • Change your mdsnippets.json file
    • See approvals/ApprovalTests.cpp@5091ab3
    • Unfortunately CLion auto-formatted which makes the diffs harder to see
    • The changes were:
      • Turn off ReadOnly
      • Remove WriteHeader line entirely Do this in Preparation stage
      • Add "Convention": "InPlaceOverwrite"
  • If your generated .md files were previously read-only, make them writable, so you can edit them
  • Review any scripts/makefiles/processes that are aware of the dependency of .md files in .source.md files (e.g. the facility to generate approvaltestscpp.readthedocs.io)

These changes are on the mdsnippet_inplace branch

@claremacrae
Copy link
Contributor

claremacrae commented Aug 18, 2020

Problem 1: mdsource/include_blah.include.md files are not found:

Error message:

Processing /Users/clare/Documents/develop/ApprovalTests/ApprovalTests.cpp/doc/UsingGoogleTests.md
Failed: Missing includes: include_ninja_warning_note.

Original line in generated .md file:

**Note:** Approval Tests has some specific build requirements if it is built with the [Ninja build generator](https://ninja-build.org/). **If you use Ninja with Approval Tests, special care is needed when setting up builds**, to avoid compilation errors or test failures. If you have any problems with this, please see [Troubleshooting Misconfigured Build](/doc/TroubleshootingMisconfiguredBuild.md#top). <!-- singleLineInclude: include_ninja_warning_note. path: /doc/mdsource/include_ninja_warning_note.include.md -->

Same for multi-line includes:

Error:

Processing /Users/clare/Documents/develop/ApprovalTests/ApprovalTests.cpp/doc/GettingStarted.md
Failed: Missing includes: include_using_test_frameworks_list.

Input:

* [Using Approval Tests With Catch](/doc/UsingCatch.md#top) <!-- include: include_using_test_frameworks_list. path: /doc/mdsource/include_using_test_frameworks_list.include.md -->
* [Using Approval Tests With Google Tests](/doc/UsingGoogleTests.md#top)
* [Using Approval Tests With Doctest](/doc/UsingDoctest.md#top)
* [Using Approval Tests With Boost.Test](/doc/UsingBoostTest.md#top)
* [Using Approval Tests With \[Boost\].UT](/doc/UsingUT.md#top) <!-- endInclude -->

@claremacrae
Copy link
Contributor

claremacrae commented Aug 18, 2020

Problem 2: https includes not found

Console output:

Failed: Missing includes: https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/fetch_content_approvaltests/mdsource/inc_fetch_content_approvaltests_cmakelists.include.md., https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/fetch_content_approvaltests/mdsource/inc_fetch_content_approvaltests_tests_cmakelists.include.md., https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/fetch_content_approvaltests_catch2/mdsource/inc_fetch_content_approvaltests_catch2_dependencies_cmakelists.include.md., https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/fetch_content_approvaltests/mdsource/inc_fetch_content_approvaltests_dependencies_cmakelists.include.md., https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/add_subdirectory_approvaltests_catch2/mdsource/inc_add_subdirectory_approvaltests_catch2_dependencies_cmakelists.include.md., https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/develop_approvaltests/mdsource/inc_develop_approvaltests_cmakelists.include.md.

Example text in generated .md file for the first of these:

 <!-- include: https://raw.githubusercontent.com/claremacrae/ApprovalTests.cpp.CMakeSamples/main/fetch_content_approvaltests/mdsource/inc_fetch_content_approvaltests_cmakelists.include.md. path:  -->

```cmake
cmake_minimum_required(VERSION 3.14 FATAL_ERROR)
# This version is needed for FetchContent_Declare & FetchContent_MakeAvailable

project(fetch_content_approvaltests)

enable_testing()

add_subdirectory(dependencies)
add_subdirectory(tests)
```
<sup><a href='https://github.com/claremacrae/ApprovalTests.cpp.CMakeSamples/blob/main/./fetch_content_approvaltests/CMakeLists.txt' title='File snippet was copied from'>snippet source</a></sup>
 <!-- endInclude -->

@claremacrae
Copy link
Contributor

claremacrae commented Aug 19, 2020

Probably you've done this already, @SimonCropp, but if the project's tests don't already test that all generated text can be round-tripped (regenerated consistently from inside a generated file), maybe that would be a good thing to build in to the tests automatically now, to ensure this keeps working for all future new features - and existing features that I haven't just tested...

@claremacrae
Copy link
Contributor

@SimonCropp This version works flawlessly - the two problems listed above are fixed. Huge thanks!

Tool 'markdownsnippets.tool' was reinstalled with the latest stable version (version '20.0.3')

/CC @isidore

@SimonCropp
Copy link
Owner

@claremacrae good to hear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants