Skip to content

Conversation

@cengelbart39
Copy link
Contributor

@cengelbart39 cengelbart39 commented May 3, 2023

Description

Resubmission of #40 following the addition of Scala Grammar in #39.

No additional changes made since #40.

Original 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

Screenshots

@cengelbart39
Copy link
Contributor Author

@thecoolwinter Resubmission following Scala Grammar addition.

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.

Leaving the same comment as on #40:

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!

@thecoolwinter thecoolwinter merged commit 9e7b7c4 into CodeEditApp:main May 3, 2023
@cengelbart39 cengelbart39 changed the title Update Build Script Update Build Script (Resubmission) May 3, 2023
@cengelbart39 cengelbart39 deleted the update/build-script branch May 3, 2023 15:56
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