👯‍♀️ Add directive option alias (#608)

You can now use `label` or `name` for all directives
jupyter-book · Sep 19, 2023 · ed7b430 · ed7b430
1 parent c873f2d
commit ed7b430
Show file tree

Hide file tree

Showing 13 changed files with 135 additions and 18 deletions.
diff --git a/.changeset/cold-ghosts-laugh.md b/.changeset/cold-ghosts-laugh.md
@@ -0,0 +1,7 @@
+---
+'myst-directives': patch
+'myst-common': patch
+'myst-parser': patch
+---
+
+Allow alias field for directive options.
diff --git a/.changeset/moody-months-cheat.md b/.changeset/moody-months-cheat.md
@@ -0,0 +1,7 @@
+---
+'myst-directives': patch
+'myst-parser': patch
+'myst-cli': patch
+---
+
+All instances of `name` options in directives can also use `label`. (e.g. in a figure or equation).
diff --git a/packages/myst-common/src/types.ts b/packages/myst-common/src/types.ts
@@ -49,7 +49,9 @@ export type ArgDefinition = {
 
 type BodyDefinition = ArgDefinition;
 
-type OptionDefinition = ArgDefinition;
+type OptionDefinition = ArgDefinition & {
+  alias?: string[];
+};
 
 export type DirectiveData = {
   name: string;

diff --git a/packages/myst-directives/src/admonition.ts b/packages/myst-directives/src/admonition.ts
@@ -25,8 +25,9 @@ export const admonitionDirective: DirectiveSpec = {
     type: ParseTypesEnum.parsed,
   },
   options: {
-    // name: {
+    // label: {
     //   type: ParseTypesEnum.string,
+    //   alias: ['name'],
     // },
     class: {
       type: ParseTypesEnum.string,

diff --git a/packages/myst-directives/src/code.ts b/packages/myst-directives/src/code.ts
@@ -10,8 +10,9 @@ export const codeDirective: DirectiveSpec = {
     type: ParseTypesEnum.string,
   },
   options: {
-    name: {
+    label: {
       type: ParseTypesEnum.string,
+      alias: ['name'],
     },
     class: {
       type: ParseTypesEnum.string,
@@ -29,7 +30,7 @@ export const codeDirective: DirectiveSpec = {
     type: ParseTypesEnum.string,
   },
   run(data): GenericNode[] {
-    const { label, identifier } = normalizeLabel(data.options?.name as string | undefined) || {};
+    const { label, identifier } = normalizeLabel(data.options?.label as string | undefined) || {};
     const numberLines = data.options?.['number-lines'] as number | undefined;
     const showLineNumbers = !!numberLines;
     const startingLineNumber = numberLines && numberLines > 1 ? numberLines : undefined;
@@ -54,8 +55,9 @@ export const codeBlockDirective: DirectiveSpec = {
     type: ParseTypesEnum.string,
   },
   options: {
-    name: {
+    label: {
       type: ParseTypesEnum.string,
+      alias: ['name'],
     },
     class: {
       type: ParseTypesEnum.string,
@@ -89,7 +91,7 @@ export const codeBlockDirective: DirectiveSpec = {
     type: ParseTypesEnum.string,
   },
   run(data): GenericNode[] {
-    const { label, identifier } = normalizeLabel(data.options?.name as string | undefined) || {};
+    const { label, identifier } = normalizeLabel(data.options?.label as string | undefined) || {};
     // Validating this should probably happen first
     const emphasizeLinesString = data.options?.['emphasize-lines'] as string | undefined;
     const emphasizeLines = emphasizeLinesString

diff --git a/packages/myst-directives/src/figure.ts b/packages/myst-directives/src/figure.ts
@@ -9,8 +9,9 @@ export const figureDirective: DirectiveSpec = {
     required: true,
   },
   options: {
-    name: {
+    label: {
       type: ParseTypesEnum.string,
+      alias: ['name'],
     },
     class: {
       type: ParseTypesEnum.string,
@@ -99,7 +100,7 @@ export const figureDirective: DirectiveSpec = {
         children.push({ type: 'legend', children: legend });
       }
     }
-    const { label, identifier } = normalizeLabel(data.options?.name as string | undefined) || {};
+    const { label, identifier } = normalizeLabel(data.options?.label as string | undefined) || {};
     const container = {
       type: 'container',
       kind: 'figure',

diff --git a/packages/myst-directives/src/iframe.ts b/packages/myst-directives/src/iframe.ts
@@ -9,16 +9,14 @@ export const iframeDirective: DirectiveSpec = {
     required: true,
   },
   options: {
-    name: {
+    label: {
       type: ParseTypesEnum.string,
+      alias: ['name'],
     },
     class: {
       type: ParseTypesEnum.string,
       // class_option: list of strings?
     },
-    label: {
-      type: ParseTypesEnum.string,
-    },
     width: {
       type: ParseTypesEnum.string,
       // length_or_percentage_or_unitless,

diff --git a/packages/myst-directives/src/image.ts b/packages/myst-directives/src/image.ts
@@ -8,8 +8,9 @@ export const imageDirective: DirectiveSpec = {
     required: true,
   },
   options: {
-    // name: {
+    // label: {
     //   type: ParseTypesEnum.string,
+    //   alias: ['name'],
     // },
     class: {
       type: ParseTypesEnum.string,

diff --git a/packages/myst-directives/src/math.ts b/packages/myst-directives/src/math.ts
@@ -6,6 +6,7 @@ export const mathDirective: DirectiveSpec = {
   options: {
     label: {
       type: ParseTypesEnum.string,
+      alias: ['name'],
     },
   },
   body: {

diff --git a/packages/myst-directives/src/table.ts b/packages/myst-directives/src/table.ts
@@ -8,8 +8,9 @@ export const listTableDirective: DirectiveSpec = {
     type: ParseTypesEnum.parsed,
   },
   options: {
-    name: {
+    label: {
       type: ParseTypesEnum.string,
+      alias: ['name'],
     },
     'header-rows': {
       type: ParseTypesEnum.number,
@@ -92,7 +93,7 @@ export const listTableDirective: DirectiveSpec = {
       }),
     };
     children.push(table);
-    const { label, identifier } = normalizeLabel(data.options?.name as string | undefined) || {};
+    const { label, identifier } = normalizeLabel(data.options?.label as string | undefined) || {};
     const container = {
       type: 'container',
       kind: 'table',

diff --git a/packages/myst-parser/src/directives.ts b/packages/myst-parser/src/directives.ts
@@ -84,8 +84,29 @@ export function applyDirectives(tree: GenericParent, specs: DirectiveSpec[], vfi
         optionNodeLookup[optionNode.name] = optionNode;
       }
     });
+
+    // Deal with each option in the spec
     Object.entries(optionsSpec || {}).forEach(([optionName, optionSpec]) => {
-      const optionNode = optionNodeLookup[optionName];
+      let optionNameUsed = optionName;
+      let optionNode = optionNodeLookup[optionName];
+      // Replace alias options or warn on duplicates
+      optionSpec.alias?.forEach((alias) => {
+        const aliasNode = optionNodeLookup[alias];
+        if (!aliasNode) return;
+        if (!optionNode && aliasNode) {
+          optionNode = aliasNode;
+          optionNameUsed = alias;
+          optionNodeLookup[optionName] = optionNode;
+        } else {
+          fileWarn(
+            vfile,
+            `option "${optionNameUsed}" used instead of "${alias}" for directive: ${name}`,
+            { node },
+          );
+        }
+        delete optionNodeLookup[alias];
+      });
+
       if (optionSpec.required && !optionNode) {
         fileError(vfile, `required option "${optionName}" not provided for directive: ${name}`, {
           node,

diff --git a/packages/myst-parser/tests/directives/directives.spec.ts b/packages/myst-parser/tests/directives/directives.spec.ts
@@ -4,11 +4,13 @@ import path from 'node:path';
 import yaml from 'js-yaml';
 import { selectAll } from 'unist-util-select';
 import { mystParse } from '../../src';
+import { VFile } from 'vfile';
 
 type TestCase = {
   title: string;
   markdown: string;
   mdast: Record<string, any>;
+  warnings?: number;
 };
 
 type TestCases = {
@@ -28,13 +30,18 @@ casesList.forEach(({ title, cases }) => {
   describe(title, () => {
     test.each(cases.map((c): [string, TestCase] => [c.title, c]))(
       '%s',
-      (_, { markdown, mdast }) => {
-        const output = mystParse(markdown);
+      (_, { markdown, mdast, warnings = 0 }) => {
+        const vfile = new VFile();
+        const output = mystParse(markdown, { vfile });
         // Dont worry about position
         selectAll('[position]', output).forEach((node) => {
           delete node.position;
         });
         expect(output).toEqual(mdast);
+        if (vfile.messages.length !== warnings) {
+          console.log(vfile.messages);
+        }
+        expect(vfile.messages.length).toBe(warnings);
       },
     );
   });

diff --git a/packages/myst-parser/tests/directives/label-alias.yml b/packages/myst-parser/tests/directives/label-alias.yml
@@ -0,0 +1,68 @@
+title: label alias tests
+cases:
+  - title: figure label works
+    markdown: |-
+      ```{figure} my_image.png
+      :label: my-fig
+      ```
+    mdast:
+      type: root
+      children:
+        - type: mystDirective
+          name: figure
+          args: my_image.png
+          options:
+            label: my-fig
+          children:
+            - type: container
+              kind: figure
+              identifier: my-fig
+              label: my-fig
+              children:
+                - type: image
+                  url: my_image.png
+  - title: figure name works
+    markdown: |-
+      ```{figure} my_image.png
+      :name: my-fig
+      ```
+    mdast:
+      type: root
+      children:
+        - type: mystDirective
+          name: figure
+          args: my_image.png
+          options:
+            name: my-fig
+          children:
+            - type: container
+              kind: figure
+              identifier: my-fig
+              label: my-fig
+              children:
+                - type: image
+                  url: my_image.png
+  - title: figure name/label warns and takes label
+    warnings: 1
+    markdown: |-
+      ```{figure} my_image.png
+      :label: my-fig
+      :name: not-used
+      ```
+    mdast:
+      type: root
+      children:
+        - type: mystDirective
+          name: figure
+          args: my_image.png
+          options:
+            label: my-fig
+            name: not-used
+          children:
+            - type: container
+              kind: figure
+              identifier: my-fig
+              label: my-fig
+              children:
+                - type: image
+                  url: my_image.png