Handle unsupported options for include directive

[JimMadge]

Closes #813

• Add warnings for unused options in non-literal include
• Add support for some/all start/end options for non-literal includes (requested in Allow non-literal include to respect start/end options #813 and Update governance and community documents to reflect current status the-turing-way/the-turing-way#4049

The filters get applied for non-literal includes. The code change is just removing a conditional block which made an early return when literal === false. This does mean includes are slightly more expensive now, and I think some of the processing of filter/options doesn't make sense when literal === false. It does keep the code quite clean and simple though.

I've tested this locally, but haven't added any tests.

# index.md
start-after and end-before

```{include} ./data.txt
:filename: true
:start-after: BEGIN
:end-before: END
```

lines

```{include} ./data.txt
:lines: 2,3
```

literal with start-after and end-before

```{include} ./data.txt
:literal: true
:start-after: BEGIN
:end-before: END
```

Test "upgrading" to literal when `lang` is defined

```{include} ./data.txt
:lang: Python
:start-after: BEGIN
:end-before: END
```

# data.txt
BEGIN
Hello
World!

*emphasis*
**bold**
END

[changeset-bot]

🦋 Changeset detected

Latest commit: 1a235b8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
myst-directives	Patch
myst-parser	Patch
myst-roles	Patch
myst-to-html	Patch
myst-transforms	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[agoose77]

@JimMadge let me know if we can be of any assistance in tackling #2 in your to-do list here!

[JimMadge]

@agoose77 That will probably be helpful. I've done a little exploring in the code and found where the Include objects are constructed, but I didn't see how/where they are processes and the AST is modified, or if this is dispatched to Sphinx.

[agoose77]

The usual pattern in mystmd is that you have a particular transform(s) that looks for a particular node type. I found one at packages/myst-transforms/src/include.ts using ripgrep:

rg -t ts -i "include\S*(directive|transform)"

The non-literal block is here:

mystmd/packages/myst-transforms/src/include.ts

Lines 124 to 126 in 0ca2c67

	const parsed = await opts.parseContent(fullFile, content);
	children = parsed.mdast.children;
	updateFrontmatterFromInclude(frontmatter, parsed.frontmatter);

[agoose77]

Taking a look at this, I think you're right that the only required change is to pass-through the filter options from the directive, i.e. you don't need to touch the transform.

The following lines define the filter:

mystmd/packages/myst-directives/src/include.ts

Lines 97 to 125 in 2299471

	const filter: Include['filter'] = {};
	ensureOnlyOneOf(data, vfile, ['start-at', 'start-line', 'start-after', 'lines']);
	ensureOnlyOneOf(data, vfile, ['end-at', 'end-line', 'end-before', 'lines']);
	filter.startAt = data.options?.['start-at'] as string;
	filter.startAfter = data.options?.['start-after'] as string;
	filter.endAt = data.options?.['end-at'] as string;
	filter.endBefore = data.options?.['end-before'] as string;
	if (data.options?.lines) {
	filter.lines = parseLinesString(
	vfile,
	select('mystDirectiveOption[name="lines"]', data.node) ?? undefined,
	data.options?.lines as string,
	);
	} else {
	const startLine = data.options?.['start-line'] as number;
	const endLine = data.options?.['end-line'] as number;
	const lines = [];
	if (startLine != null) lines.push(startLine);
	if (startLine == null && endLine != null) lines.push(0);
	if (endLine != null) lines.push(endLine);
	if (lines.length > 0) {
	filter.lines = [
	lines.map((n) => {
	if (n >= 0) return n + 1;
	return n;
	}) as [number, number?],
	];
	}
	}

[JimMadge]

Taking a look at this, I think you're right that the only required change is to pass-through the filter options from the directive, i.e. you don't need to touch the transform.

🎉 That was going to be the first thing I tried hoping it worked 😆.

I can see that the code essentially says "if not literal ignore all options, otherwise apply options" so I was hoping I could simply change that 👍.

[agoose77]

That appears to be the case! Are you happy adding those changes to this PR?

[JimMadge]

That appears to be the case! Are you happy adding those changes to this PR?

Yep, I'll get on to that 🚀.

[JimMadge]

I'm getting the same test errors on main as on this branch, so I think it isn't related to my changes.

[JimMadge]

Perhaps caption is also makes no sense for a non-literal include?

mystmd/packages/myst-directives/src/include.ts

Line 119 in cf18f20

caption: data.options?.caption as any[],

Would this work?

      caption: literal ? data.options?.caption as any[] : [],

[agoose77]

@JimMadge I've fixed the tests — this PR broke them because the test fails if the new AST has attributes that the expected AST does not.

Regarding caption, I agree we should hide it. I think instead of setting to [], let's pull out caption as a local variable and set it to undefined for literal === true.

[JimMadge]

Ah, I think I must have forgotten to build after checking out main 🤦

[JimMadge]

I think these changes would mean a patch release from my reading of https://github.com/jupyter-book/mystmd/blob/main/CONTRIBUTING.md