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

Experimental REPL PoC #136

Merged
merged 1 commit into from Jun 14, 2022
Merged

Experimental REPL PoC #136

merged 1 commit into from Jun 14, 2022

Conversation

jpschorr
Copy link
Contributor

@jpschorr jpschorr commented Jun 11, 2022

This is a minimal PoC for the start of a REPL. It should be considered experimental, subject to change, etc.

I wrote this in order to test the parser interface & types from the perspective of an external application. As a result, some minor tweaks were made around error reporting. Potentially a refactor of the mietter::Diagnostic stuff in this PR will be included in the parser crate in the future.

It currently accepts no commands, assuming any/all input is a PartiQL query, which it will attempt to parse. Parse errors are pretty printed to the output.

This uses:

  • miette for errors
  • rustyline for readline/editing/history
  • syntect for syntax highlighting via the included sublime ion and partiql syntax definitions

Demo:
demo

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Jun 11, 2022

Codecov Report

Merging #136 (e10b42a) into main (1aae4b4) will decrease coverage by 0.12%.
The diff coverage is 64.51%.

@@            Coverage Diff             @@
##             main     #136      +/-   ##
==========================================
- Coverage   87.85%   87.73%   -0.13%     
==========================================
  Files          20       20              
  Lines        2257     2258       +1     
==========================================
- Hits         1983     1981       -2     
- Misses        274      277       +3     
Impacted Files Coverage Δ
partiql-parser/src/error.rs 78.68% <0.00%> (+0.91%) ⬆️
partiql-parser/src/lib.rs 52.63% <42.85%> (-8.91%) ⬇️
partiql-parser/src/parse/mod.rs 94.35% <80.00%> (-0.06%) ⬇️
partiql-parser/src/preprocessor.rs 93.01% <100.00%> (ø)
partiql-parser/src/token_parser.rs 98.27% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1aae4b4...e10b42a. Read the comment docs.

@jpschorr jpschorr requested review from almann, am357 and alancai98 and removed request for almann June 11, 2022 05:33
@github-actions
Copy link

github-actions bot commented Jun 11, 2022

Conformance comparison report

main (1aae4b4) 75bb1d0 +/-
% Passing 96.93% 96.93% 0.00%
✅ Passing 158 158 0
❌ Failing 5 5 0
Total Tests 163 163 0

Number passing in both: 158

Number failing in both: 5

Number passing in main but now fail: 0

Number failing in main but now pass: 0

Copy link
Contributor

@am357 am357 left a comment

Choose a reason for hiding this comment

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

Left some comments, would also be great to add more documentation to partiql-cli.rs.

partiql-cli/src/bin/partiql-cli.rs Outdated Show resolved Hide resolved
partiql-cli/src/bin/partiql-cli.rs Show resolved Hide resolved
partiql-cli/src/bin/partiql-cli.rs Show resolved Hide resolved
.iter()
.any(|err| matches!(err, ParseError::UnexpectedEndOfInput))
{
Ok(ValidationResult::Incomplete)
Copy link
Contributor

Choose a reason for hiding this comment

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

From couple errors I've seen, this can be as a result of incomplete Grammar, do we want to provide some hints or perhaps link for cutting an issue if the input appears to be correct. For example before this PR we were getting UnexpectedEndOfInput for SELECT * FROM a AS a CROSS JOIN c AS c AT q.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sure to bring this out of PoC we will need something more robust here. For now, this is what allows you to do things like hit <ENTER> and continue writing the query on the next line in the middle of a query.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair, maybe as a follow-on we can add a README to emphasize on the PoC.

Copy link
Member

Choose a reason for hiding this comment

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

+1 a short README would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added minimal README

partiql-cli/src/bin/demo.yml Outdated Show resolved Hide resolved
@jpschorr jpschorr force-pushed the spike-cli branch 2 times, most recently from ed0726d to ce91826 Compare June 14, 2022 18:56
Copy link
Member

@alancai98 alancai98 left a comment

Choose a reason for hiding this comment

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

Very cool! I especially like the syntax highlighting along with the syntax error underlining, which makes it easier to see where exactly an error occurred.

Left a non-blocking comment related to the long-term location for the sublime syntax files.

@@ -0,0 +1,131 @@
%YAML 1.2
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious if there a better place to store these syntax files so they could be used by other programs. Is there a way to publish the sublime syntaxes (i.e. partiql.sublime-syntax and ion.sublime-syntax) somewhere similar to what you had done for the ACE editor (ajaxorg/ace#4630)? Alternatively, we could have a separate repo for the syntax highlighting modes that could be taken as a submodule of this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We should likely have all the tooling somewhere else and bring it in via submodule or something else here. The official way add a sublime extension is via https://packagecontrol.io/docs/submitting_a_package

.iter()
.any(|err| matches!(err, ParseError::UnexpectedEndOfInput))
{
Ok(ValidationResult::Incomplete)
Copy link
Member

Choose a reason for hiding this comment

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

+1 a short README would be helpful.

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.

None yet

3 participants