Skip to content
This repository was archived by the owner on May 15, 2023. It is now read-only.

Conversation

@ntkme
Copy link
Contributor

@ntkme ntkme commented Aug 19, 2021

Closes #31.

Although the embedded protocol can provide this information via VersionRequest, there is a chicken egg problem:

  • Without making a VersionRequest, host doesn't know the correct protocol_version to use to talk to embedded.
  • Without knowing the correct protocol version to use, a VersionRequest may fail. This is very unlikely, but could happen in theory.

Therefore it is helpful to implement a --version flag in the cli interface that prints the same information as VersionReponse:

$ ./build/dart-sass-embedded --version
{
  "protocol_version": "1.0.0-beta.12",
  "compiler_version": "1.0.0-beta.9",
  "implementation_version": "1.38.0",
  "implementation_name": "Dart Sass"
}

This would allow the host implementation to check and handle compatibility issues better. Host could also download the proper version of proto file and recompile as needed, etc.e

@nex3 nex3 self-requested a review August 23, 2021 22:44
@nex3
Copy link
Contributor

nex3 commented Aug 23, 2021

I like the idea here, but there are a couple things I'd like to see:

  • Rather than coming up with a custom JSON format, emit the JSON encoding of the VersionResponse proto (with id set to 0).
  • Describe the behavior of this flag in the readme.

@ntkme
Copy link
Contributor Author

ntkme commented Aug 24, 2021

Updated to print JSON representation of versionResponse.

$ ./build/dart-sass-embedded --version
{
  "protocolVersion": "1.0.0-beta.12",
  "compilerVersion": "1.0.0-beta.9",
  "implementationVersion": "1.38.0",
  "implementationName": "Dart Sass",
  "id": 0
}

@nex3
Copy link
Contributor

nex3 commented Aug 28, 2021

Thanks for your contribution! I'll merge this once I update the compiler to know about Value.HwbColor.

@nex3 nex3 merged commit d206351 into sass:main Sep 2, 2021
@ntkme ntkme deleted the cli-version-flag branch September 2, 2021 01:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Versionning for the implemented protocol

2 participants