-
Notifications
You must be signed in to change notification settings - Fork 868
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
[Proposal] Simplify snippet inclusion from external files #4677
Comments
Have we given much thought to the impact of this on the build pipeline (@herohua )? The system is flying along, building markdown files and then boom it hits one of these and has to go fetch/download/sync an external repo. Or is the thought that it would just go fetch that one file from GitHub vs. pulling down the repo? |
@DuncanmaMSFT the goal is to avoid repo clones altogether - we just need that one raw file. Granted, there likely would be a perf impact if we are pulling a 100MB file, but that would be a huge deviation from the goal of the code snippet feature. |
Cool, and are we assuming that file is available publically without auth, or is that external to the scope of the feature? |
@DuncanmaMSFT starting with public code with no auth. Protected snippets, to our scenario, are an edge case - we aim to ensure that all sample code integrated in docs is public. |
With the current requirement of the config file edit, we have some governance over whether to enable pulling in code, and from where. This feature reduces the friction by ditching the config file edit requirement, but might be considered riskier security-wise as anyone can pull any code in from wherever and have it published. If I'm Evil Actor and I want Trusting Public to copy my nefarious code and run it, I could slip it in to a doc from my own repository and have it published. |
The @mmacy the other concern to is what happens if the repo code changes later after the reference is applied and the doc is republished and just takes the new changes. It should probably have to reference a specific commit to maintain consistency so the document has to be updated to match changes to the source still? |
We should also make sure the custom syntax doesn't cause common markdown parsers to fail so that documentation can still be read in a markdown reader/editor. (E.g. Visual Studio Code, GitHub, Windows Community Toolkit Markdown Control, etc...) |
@michael-hawker Specifying a commit would be a nice-to-have, but one of the primary benefits of remote code inclusion using regions (as opposed to line numbers or a specific commit) is so that the code can be updated in an out-of-band fashion (such as to accommodate to a new library version) without also having to update the article. |
Is there any specific reason for not treating |
How about an alternative:
Benefits:
|
@DuncanmaMSFT , it does have some impact on the build pipeline, downloading just the url makes the impact manageable. For authentication, it can go through the same pipeline we use to clone git and download other files. |
Thanks to @fbartho and @KalleOlaviNiemitalo, force every property on a new line, replace
|
An array of repeated hashes for code snippets embedded in triple-backticks would work for a code sets too! |
If you replace the equals signs with colons, too. |
If you put triple tildes or quadruple backticks around the Markdown example in the comment, you don't have to break the triple backtick with a space. https://spec.commonmark.org/0.28/#example-92 |
dotnet/try uses following syntax: # My Interactive Document:
```cs --source-file ./myApp/Program.cs --project ./myApp/myApp.csproj
``` However, this syntax renders some wield empty blocks under normal Markdown preview. See: https://github.com/dotnet/try/blob/master/docs/test_example/lesson.md I'd vote for the more informative YAML syntax. |
I love the idea to include regions or line ranges! better yet if something like I'd like to be able to include a commit in the reference, to get alerted/notified when the base code has changed, so you could verify if the related text is still relevant for the code (or viceversa). This is useful for guides like the Microservices e-book |
Lots of great feedback, folks - thank you! I will address each point separately. @mmacy regarding your comment:
It's a fair assertion, and it will be another guideline that we will need to have in place to import code. For clarity's sake, I should mention that on docs.microsoft.com, we should have an allow-list of domains from which the code can be imported. @michael-hawker yes, referencing by commit should be the behavior if you want to preserve the specific point-in-time snapshot of your code. For example, one can just use a link similar to the following:
Fair point about editability and previews. Because this is a Markdown extension, this is, by-design, non-standard Markdown that is going to be hard to render in "vanilla" scenarios, such as GitHub or VSCode. That said, this is something for us to consider to integrate in the Docs Authoring Pack. @ahmadawais good point - it should just work on a new line (see containers spec). I am using that for brevity. @fbartho @KalleOlaviNiemitalo interesting idea, my hesitation there being that we would have a third way of doing things in DocFX, which is generally not recommended. Specifically, we already have a number of Markdown extensions implemented that use the @mvelosop that's a good one - indication of "space" between ranges might be useful for customers. I like that. Can you elaborate more on the "commit in the reference" point? |
I have roughly 0 experience in this repository / documentation system, I've only consumed the docs in a browser. So please take my suggestion with a grain of salt! I joined this thread because of a random twitter discussion that I saw that linked here. However, @dendeli-msft, I do have experience with using several markdown previews simultaneously on a same project. For example: seeing the preview in my editor, seeing the preview in one of several dedicated markdown previewers, and seeing the preview in GitHub-markdown (literally have been dealing with this all week for several different projects). Without knowing about that prior-art for My personal objection would be withdrawn if GitHub ended up handling these cleaner & extended the Markdown spec to support that flavor.
|
This empty code fences are for the local workshop / content authoring mode, where the code is pulled in from a backing project while the |
@fbartho this is great feedback, we really appreciate it. What if the extension would effectively follow a mix of what @superyyrrzz provided, along with the current convention? Something like:
@jonsequitur - what do you think of the above? |
That seems fine to me! I don't specifically have any strong opinions about how the configuration flags are passed. If you support multiline mode, then you could get a semi-documented fallback:
Looks like: src="https://foo.bar/test.cs" range="1-2,6-53" In vanilla renderers, which might be a good fallback! |
@fbartho the only drawback I can think of with multi-line is that it is easier to confuse that with an actual code snippet. For example, because GitHub does not support the custom functionality that we have, it rendered it as the snippet you've shown above. Me, a reader that has no idea what's going on behind the scenes, might think that you are providing me a literal code piece. |
We considered line ranges in Try .NET and decided against it because they're not stable. In C#, the |
@jonsequitur for DocFX, we got the region problem solved in-house: Tag Name Representation in Code Snippet Source File. Fair concern with line numbers. That's why the guideline for us would be that if anyone uses line numbers and doesn't want the content to change if the code changes, they can either rely on regions or reference the file from a specific Git commit. |
I would worry authors might not immediately understand the risk of using the line ranges and maybe the capability carries an inherent risk of fragility that's better avoided from the outset. |
@jonsequitur that's fair feedback - have you encountered cases where scenarios that required lines were not well-addressed with regions? |
Regions work great for C#, because they're part of the compiler's semantic model. Going through the compiler enables subtleties like the indentation being adjusted when code is rendered into the editor, and diagnostic squiggles are then positioned correctly within the reformatted code. This is not easy to do if you're treating the code as simple text. Semantics also mean verification can be smarter. Try .NET knows the difference, for example, between an error that the user typed versus an error that the content author typed in the hidden portion of the code. Comments should also work for most languages. Another convention we've considered is to reference code snippets by type and method name. This is more limiting. Both of these favor semantics over coordinates. On a separate note, in most cases, interactivity needs additional parameters, and the parameters needed will differ from language to language. Some can be supplied inline with the snippet but others, to avoid having to maintain duplicate copies of the content differing by only a parameter or two, should be able to be supplied at a different level than the content. Examples might include runtimes, language versions, and package dependency versions. |
Hi @dendeli-msft. it looks like there are at least two use cases regarding region/line ranges:
Most of what I do in docs is for number 2, because there's some text explanation related to the code shown (for context, related to eShopOnContainers and the Microservices Architecture Guide). That can be solved by referencing the code by commit, as you point out above. So far so good. However... At some point, someone else updates the code (e.g. upgrade from .NET Core 2.2 to 3.0) and even though the guide would still show the original code, it'd not be "current" and the related text might also need to be updated. So, it'd be great to have some sort of warning that the referenced code file has been changed, so someone can take a look and decide whether to update the commit reference, because no change is needed or rewrite a whole section because whatever doesn't hold true any more. Since this is the "day-dreaming" phase 😉 it'd also be great to have:
Thanks! |
Today, when writing an article on docs.microsoft.com and needing to embed some code snippets that are coming from another repository, the writer needs to go through a convoluted process of updating the repository configuration and then using relative references to the code sample (see example of current dependent repository chain).
This is a time-consuming process that is error-prone (the user can accidentally mis-configure the repo) and has the potential of affecting other content (if the configuration was incorrectly updated). To reduce the friction in adding new code samples, we are proposing a new convention, by which embedding code is much more efficient and intuitive.
Baseline Experience Goals
The experience for referencing code from an external repository should be:
For each code snippet that is inserted into docs.microsoft.com, we need to account for several additional capabilities:
Authoring Experience
Referencing code should be done through a new extension, designed to match the experience we’ve put together for Markdown extensions on docs.microsoft.com – using the triple-colon demarcation (
:::
). This is the convention defined in the Markdig documentation for custom containers.The extension name should be
code
.The user can include plain-text code within the extension, without having to import code from a file.
lang
Example:
lang=”csharp”
src
Example:
src=” https://raw.githubusercontent.com/MicrosoftDocs/DenDeli-Test/master/dendelinet/docfx.json”
region
Example:
region=”myAwesomeCodeSample”
highlight
range
Example:
range=”1-5,9,12-4”
disableLinkToSource
Example:
disableLinkToSource=”true”
interactive
Example:
interactive=”false”
title
Example: `title="Something awesome going on"
Following the conventions above, a fully defined code entry in a Markdown file might look like the following:
To present two code snippets that belong to different languages, and are interchangeable, the code blocks need to be wrapped in a
codeset
element, as such:This will ensure that both code snippets are treated as the same, just implemented in a different language, rather than showing two separate code samples.
A
codeset
element does not have any attributes that the user can set. If acodeset
element only has one code snippet presented, the language switcher should not be displayed, and the code snippet is treated as a standalone code entity.Status
The text was updated successfully, but these errors were encountered: