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

Includes Markdown AST Metadata into non-code Cells #388

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

pastuxso
Copy link
Collaborator

@pastuxso pastuxso commented Oct 2, 2023

Metadata sample output:

{
  "Children": [
    {
      "Kind": "Text",
      "Text": "Paragraph 1 with a link "
    },
    {
      "Children": [
        {
          "Kind": "Text",
          "Text": "Link1"
        }
      ],
      "Destination": "https://example.com",
      "Kind": "Link",
      "Title": "Link Title 1"
    },
    {
      "Kind": "Text",
      "Text": " and a second link "
    },
    {
      "Children": [
        {
          "Kind": "Text",
          "Text": "Link2"
        }
      ],
      "Destination": "https://example2.com",
      "Kind": "Link",
      "Title": "Link Title 2"
    },
    {
      "Kind": "Text",
      "Text": ".."
    }
  ],
  "Kind": "Paragraph",
  "RawText": "Paragraph 1 with a link [Link1](https://example.com 'Link Title 1') and a second link [Link2](https://example2.com 'Link Title 2').."
}

@pastuxso pastuxso force-pushed the cepeda/markdown-metadata branch from e43b9ec to 97b3836 Compare October 2, 2023 19:30
@sourishkrout sourishkrout changed the title (wip) adds markdown metada (wip) adds markdown metadata Oct 5, 2023
@pastuxso pastuxso force-pushed the cepeda/markdown-metadata branch 3 times, most recently from d1caec3 to deb04f5 Compare October 10, 2023 19:09
@sourishkrout
Copy link
Member

I'm undecided on using the runme.dev/ prefix for e.g. "runme.dev/3/Kind". We introduce it to pass additional information between extension <> kernel that's exempt from serialization. The metadata list is getting a bit busy using the same exact mechanism. Have you tried grouping the AST information under a single property, such as runme.dev/ast and just nest them @pastuxso?

@pastuxso
Copy link
Collaborator Author

pastuxso commented Oct 17, 2023

@sourishkrout, I believe we could implement a nested structure, though it would necessitate significant modifications to our existing one. Alternatively, we could incorporate a stringified JSON under the key runme.dev/ast, allowing us to maintain the map[string]string format for metadata.

What do you think about the following structure?

{
  "runme.dev/ast": {
    "Kind": "Paragraph",
    "RawText": "Some content with [Link1](https://example1.com \"Link title 1\") [Link2](https://example2.com \"Link title2\")",
    "Children": [
      {
        "Kind": "Text",
        "Text": "Some content with "
      },
      {
        "Destination": "https://example1.com",
        "Kind": "Link",
        "Title": "Link title 1"
      },
      {
        "Kind": "Text",
        "Text": " "
      },
      {
        "Destination": "https://example2.com",
        "Kind": "Link",
        "Title": "Link title2"
      },
      {
        "Kind": "Text",
        "Text": "Link1"
      },
      {
        "Kind": "Text",
        "Text": "Link2"
      }
    ]
  }
}

@sourishkrout
Copy link
Member

@pastuxso I like it better 👍 . Scopes/namespaces it under a single property.

@pastuxso pastuxso force-pushed the cepeda/markdown-metadata branch 2 times, most recently from 7b35af6 to 0ae8a59 Compare October 18, 2023 15:06
@sourishkrout
Copy link
Member

Let's figure out what's left to get this merged. We should probably just have a convo (after standup) or something about it.

@pastuxso pastuxso changed the title (wip) adds markdown metadata Includes Markdown AST Metadata into non-code Cells Oct 18, 2023
@pastuxso pastuxso marked this pull request as ready for review October 18, 2023 16:09
@pastuxso pastuxso force-pushed the cepeda/markdown-metadata branch 2 times, most recently from 46ec3f3 to f95881b Compare October 20, 2023 15:22
@sourishkrout sourishkrout self-requested a review October 20, 2023 15:45
@pastuxso pastuxso force-pushed the cepeda/markdown-metadata branch from f95881b to 6efcce9 Compare October 20, 2023 16:54
Copy link
Member

@sourishkrout sourishkrout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM. I saw some minor issues inside the AST info. However, let's merge for now and circle back later.

@pastuxso pastuxso merged commit 19b53d2 into main Oct 23, 2023
4 checks passed
@pastuxso pastuxso deleted the cepeda/markdown-metadata branch October 23, 2023 15:00
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.

3 participants