Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Conversation

@robrix
Copy link
Contributor

@robrix robrix commented Jun 5, 2019

This PR bumps the parsers to master, except for tree-sitter-python, which is on the node-fields branch, and tree-sitter-typescript which is reset back to what we had it at before tree-sitter/haskell-tree-sitter#86 accidentally changed it. That means we’re now using a recent haskell-tree-sitter and can work at migrating it & the language packages to hackage!

Copy link
Contributor Author

@robrix robrix left a comment

Choose a reason for hiding this comment

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

Okay, there are still some tagging test failures which @patrickt is giving me a hand with but this is otherwise ready for review.

Fixed! Ready for review.

block = symbol Block *> children (makeTerm'' <$> location <*> manyTerm expression)

block' :: Assignment Term
block' = symbol Block *> children (makeTerm <$> location <*> manyTerm expression)
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 defined these to incur as few changes to the fixtures as possibly; basically I only changed the fixtures where the assignments themselves were outright wrong.

withItem = symbol WithItem *> children (flip Statement.Let <$> term expression <*> term (expression <|> emptyTerm))
<|> flip Statement.Let <$> term expression <*> emptyTerm
make (val, name) = makeTerm1 . Statement.Let name val
withItem = symbol WithItem *> children ((,) <$> term expression <*> term (expression <|> emptyTerm))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This had been incorrectly building a sequence of separate Lets instead of a right-nested chain of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

children (flip (foldr make) — that's the Good Shit. that's the Shit I Like. Hell yeah.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes parsing is Good, Actually.

tryStatement :: Assignment Term
tryStatement = makeTerm <$> symbol TryStatement <*> children (Statement.Try <$> term expression <*> manyTerm (expression <|> elseClause))
where elseClause = makeTerm <$> symbol ElseClause <*> children (Statement.Else <$> emptyTerm <*> expressions)
tryStatement = makeTerm <$> symbol TryStatement <*> children (Statement.Try <$> term block <*> manyTerm (expression <|> elseClause))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

try statements with multiple statements in the body were shoving all but the first into the list of catches. Whoops!

, (inject .) . Expression.Minus <$ (symbol AnonMinus <|> symbol AnonMinus' <|> symbol AnonMinus'')
, (inject .) . Expression.Times <$ (symbol AnonStar <|> symbol AnonStar')
, (inject .) . Expression.Power <$ symbol AnonStarStar
, (inject .) . Expression.DividedBy <$ (symbol AnonSlash <|> symbol AnonSlash' <|> symbol AnonSlash'')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty trivial; just removing some symbols that don’t exist any more.

result <- if compatible then
liftIO $ runParser parser blobSource
else
Failed <$ trace "tree-sitter: incompatible versions"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually we should do something smarter than this (because we still report this as a parser timeout exception), but for now at least it’s distinguishable by the trace.

actual `shouldBe` expected
actual <- fmap verbatim <$> parseFilePath session path
case actual of
Left err -> throw err
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This lets us see the actual exception instead of just ExitFailure or w/e.

{+(Catch
{+(Identifier)+}
{+(Statements)+})+}
{-(Identifier)-}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are you doing here (Identifier)? You belong in the body!

{-(Assignment
{-(Identifier)-}
{-(Boolean)-})-})-})-}))
{+(Statements
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just un-nested a single layer of [].

, Language.JSON.Assignment
, Language.JSON.PrettyPrint
, Language.MiniRuby.Assignment
, Language.MiniPython.Assignment
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were pretty much just dead weight. 🔥 with much 🔥ing.

@robrix robrix requested a review from a team June 6, 2019 15:48
Copy link
Contributor

@patrickt patrickt left a comment

Choose a reason for hiding this comment

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

Bravo, @robrix. Bravo.

withItem = symbol WithItem *> children (flip Statement.Let <$> term expression <*> term (expression <|> emptyTerm))
<|> flip Statement.Let <$> term expression <*> emptyTerm
make (val, name) = makeTerm1 . Statement.Let name val
withItem = symbol WithItem *> children ((,) <$> term expression <*> term (expression <|> emptyTerm))
Copy link
Contributor

Choose a reason for hiding this comment

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

children (flip (foldr make) — that's the Good Shit. that's the Shit I Like. Hell yeah.

@patrickt patrickt merged commit e9e9c0e into master Jun 6, 2019
@patrickt patrickt deleted the fail-all-of-the-tests-at-once-and-explode-into-space branch June 6, 2019 18:59
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.

3 participants