Skip to content

Conversation

@cengelbart39
Copy link
Contributor

Description

This PR addresses issues when adding tree-sitter-markdown and running the build script with it.

This tree-sitter features a split parser (essentially two tree-sitters in one repo), one for Markdown and the other for Markdown Inline. Since each parser contains separate highlight and injection files of the same name, the current build script will fail when copying these files.

This PR addresses this by using swift package dump-package and jq to get target info. Particularly, the path property of each target and the number of targets.

Any single-target packages, output exactly the same.

With tree-sitter-markdown, the paths go to the tree-sitter-markdown and tree-sitter-markdown-inline directories respectively. These paths become the name of directories created in Sources/CodeEditLanguages/Resources and copy the correct files over.

The are some tree sitters, like tree-sitter-typescript, that, despite having multiple targets, the path properties for every target points to the root directory. In this case, it is treated as a single-target package.

All currently supported languages pass through the build script properly.

I also updated the "Add Language" and "Update Language" documentation to reflect the new dependency on jq.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

@cengelbart39 cengelbart39 mentioned this pull request May 1, 2023
6 tasks
Copy link
Contributor

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

I'm initially hesitant to add the jq requirement just to fix this problem but I think it's okay here due to the fact that it doesn't affect CI. Well done!

@cengelbart39
Copy link
Contributor Author

Thanks!

@thecoolwinter
Copy link
Contributor

Ah looks like #39 caused a conflict with the container zip. Could you pull from main and I'll approve and merge asap.

@cengelbart39
Copy link
Contributor Author

Gonna have to resubmit since I had to disregard all the commits.

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.

2 participants