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

Publish parser artifacts on GitHub pages build #100

Merged
merged 2 commits into from
Mar 9, 2023
Merged

Conversation

DerekStride
Copy link
Owner

What

This is an alternative to #90. cc/ @matthias-Q @dmfay

This PR adds a jekyll site in the docs/ folder of the repository. When new changes are merged into the main branch the gh-pages.yml workflow runs, it will:

  1. generate the parser artifacts
  2. copy them into docs/src/
  3. generate the site
  4. publish it to the gh-pages branch.

This keeps the PR merge workflow the same as before and keeps the main branch history scoped to changes to the grammar & tests. The parser artifacts will still exist in the gh-pages branch as well as be hosted on the generated site.

Preview

Landing page

Screenshot 2023-02-17 at 4 37 50 PM

src/ directory

Screenshot 2023-02-17 at 4 38 00 PM

@matthias-Q
Copy link
Collaborator

Does this mean, that downstream users need to know where to look for the parser.c? AFAIK nvim-treesitter can either check the src/parser.c directly, or has an option to rebuild it (which requires npm)

So instead of using src/parser.c it will use docs/src/parser.c, correct?

@DerekStride
Copy link
Owner Author

AFAIK nvim-treesitter can either check the src/parser.c directly, or has an option to rebuild it (which requires npm)

This change would require nvim-treesitter to rebuild the parser. I have to check helix to see if that type of behaviour is supported or not.

So instead of using src/parser.c it will use docs/src/parser.c, correct?

The file doesn't actually get committed to the repo with the proposed change. The file could be found in src/parser.c on the gh-pages branch though.

We could git rid of the github pages part altogether and just publish generated files on a separate branch e.g. parser-artifacts.

@DerekStride DerekStride force-pushed the ds/gh-pages branch 4 times, most recently from b530d6f to c2c833a Compare March 2, 2023 22:07
@DerekStride
Copy link
Owner Author

I've setup a sample repository and deployed it to github pages to validate my idea.

You can find the webpage here: https://derek.stride.host/tree-sitter-sql-test/
This is the gh-pages branch with the parser artifacts: https://github.com/DerekStride/tree-sitter-sql-test/tree/gh-pages
The is the main branch without all the generated files: https://github.com/DerekStride/tree-sitter-sql-test/tree/main

This repo would mirror the behaviour of that repo just with the site published to: https://derek.stride.host/tree-sitter-sql (currently 404s until this PR is shipped).

Considerations

I've checked both helix & nvim-treesitter, I don't think either would be affected by this change. We'd just need to point helix at a revision from the gh-pages branch.

@matthias-Q @dmfay Does this seem like an okay solution to the both of you?

@LeoniePhiline does my assessment of the effect on Helix look correct?

@matthias-Q
Copy link
Collaborator

I think this is a good idea. For nvim-treesitter, I would still add the branch in their parsers.lua even though it will be generated & compile upon running :TSInstall

For helix, I am not so sure. Language support is defined in the languages.toml under the section grammar. This just takes a git, rev and subpath.
https://docs.helix-editor.com/languages.html
Will the rev point to the correct location?

@dmfay
Copy link
Collaborator

dmfay commented Mar 3, 2023

Node projects using it as a dependency should be okay since they'll run the install lifecycle script automatically; removing that was the big issue with the first stab at this problem. I don't know enough about how the various editors integrate grammars to be able to say there though (do we need to worry about vscode at all? they also use tree-sitter iirc).

@matthias-Q
Copy link
Collaborator

VSCode uses the Textmate. There is a different tree-sitter-sql grammar listed on the official tree-sitter github page. I think that newer/other editors will use that when they implement TS (e.g. zed editor, but I do not know for sure).

I would argue, that we also should make that clear in the main Readme.md.

@DerekStride
Copy link
Owner Author

I've done some testing for nvim-treesitter & helix and I'm still fairly confident they'll be able to work with the new model.

nvim-treesitter should work with the custom branch name, I created a manual entry in the parser config for neovim & verified it installed correctly.

local parser_config = require "nvim-treesitter.parsers".get_parser_configs()

parser_config["sql-test"] = {
  install_info = {
    url = "https://github.com/DerekStride/tree-sitter-sql-test",
    branch = "gh-pages",
    files = { "src/parser.c" },
  },
}

For helix I wasn't as thorough but I did go through the code and replicate their process of fetching the grammar:

$ git clone --depth=1 https://github.com/DerekStride/tree-sitter-sql-test
Cloning into 'tree-sitter-sql-test'...
$ cd tree-sitter-sql-test
$ git fetch --depth=1 origin 6a73f0d4e680e84bfd16edb8807e67cb14578d76
$ git checkout 6a73f0d4e680e84bfd16edb8807e67cb14578d76
$ ls -T .
drwxr-xr-x    - derek ./
drwxr-xr-x    - derek ├── src/
drwxr-xr-x    - derek │  ├── tree_sitter/
.rw-r--r-- 5.4k derek │  │  └── parser.h
.rw-r--r-- 242k derek │  ├── grammar.json
.rw-r--r-- 115k derek │  ├── node-types.json
.rw-r--r-- 8.9M derek │  └── parser.c
.rw-r--r-- 1.1k derek └── index.html

@DerekStride
Copy link
Owner Author

Let's give this a try and if projects encounter issues with this setup we can revert & re-evaluate. I'll work on a follow-up PR to improve our README and explain the project structure.

@DerekStride DerekStride merged commit fcc9a44 into main Mar 9, 2023
@DerekStride DerekStride deleted the ds/gh-pages branch March 9, 2023 17:47
DerekStride added a commit that referenced this pull request Nov 9, 2023
In #100 we removed the generated files an publish them via GitHub pages.
Since then we've added a custom src/scanner.c file that is being treated
as a generated file on GitHub due to the .gitattributes file. Since it's
no longer needed, I've removing it.
@DerekStride DerekStride mentioned this pull request Nov 9, 2023
Hdoc1509 added a commit to Hdoc1509/tree-sitter-hygen-template that referenced this pull request Oct 2, 2024
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