Engine separation keep playground #672
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This includes moving the sample tests to Topiary cli (because topiary no longer knows about languages). Additionally, bash was reverted back to an older version due to a regression that needs to be investigated.
At this point, the LanguageDefinition contained only a single value, and was thus mostly useless
ErinvanderVeen
force-pushed
the
engine-separation-keep-playground
branch
from
8f4d1b4 to
550cba9
Compare
ErinvanderVeen
force-pushed
the
engine-separation-keep-playground
branch
2 times, most recently
from
173be1a to
5efa44a
Compare
added 2 commits
January 25, 2024 14:05
See the comment in that module for more information.
But only if not in wasm
ErinvanderVeen
force-pushed
the
engine-separation-keep-playground
branch
from
5efa44a to
df18e4b
Compare
|
Presumably this supersedes #643 |
Absolutely, but I want to keep it until we are sure we can merge this. |
Xophmeister
approved these changes
Jan 26, 2024
Small typos
This was referenced Jan 26, 2024
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An attempt to implement #643 without having to deprecate the playground.
The main idea here is to vendor the git crates that we absolutely depend on for the Topiary, and eventually publishing them under the
topiary-*crate.Since this PR is very large, I want to reason about all the changes I have made:
Clippy CI phase not running on MacOS
Unfortunately there seems to be something in our Nix config/Cargo.toml that makes
cargo clippyattempt to try and build the playground on MacOS. This, in combination with a undetermined bug with building the playground on MacOS, led me to disable it for now.Building the playground on MacOS manually
nix build .#topiary-playgroundworks fine.Removing the .vscode directory
I don't think .vscode has a place in an otherwise editor agnostic repository.
Removing the patch versions of some of our packages
Generally, Cargo will favour newer versions over older versions anyway. Assuming the packages abide to SemVer (even below 1.0) it should therefore generally not be a problem to not specify the patch level. In some cases this might ensure that we don't build duplicate versions of packages
removing the
SupportedLanguagesenumThis was done to make separating the
topiary-configcrate easier. With eyes on the future, this will also make it easier for us to maker it easier for external contributors to add language support to Topiary.new DevShell
I got annoyed I could not build the Topiary Playground locally with
cargo, so I exposed a second devShell namedwasmto build wasm targets. Ideally, we would find a way to combine these.Error changes
With
topiary-configbecoming its own crate, it needed its own errors. With this, I changed some of our Errors to be more in line with what is common in the Rust ecosystem, with eyes on thethiserrorcrate in the future.Moving the samples and using the cli to test them
Since the core
topiarycrate no longer depends on all thetree-sittergrammars, this crate could no longer be responsible for the testing. All the configuration was already done in thetopiary-clicrate, so I moved the sample testing there.Vendoring the
tree-sitter-*cratesWe are moving to a moment where we publish
topiary-coreon crates.io. With this move we need to publish updated version ofweb-tree-sitter-sysandtree-sitter-facade. We could ask for ownership of the crates, but generally, I like prefixing them withtopiary-.This prevents people from assuming they are useful for anything else.
We might in the future ask for ownership and maintain them, but we don't have the manpower for that right now.
npm updateThe puppeteer package that we use uses a package called
browsersto install specific versions of chrome. Unfortunately, Google made the specific version of chrome that we used unavailable. This was long fixed upstream, so a simplenpm updateresolved the issue.