Skip to content

Faulty code: improve error block node#13264

Merged
MarcusDenker merged 4 commits intopharo-project:Pharo12from
privat:faulty-code-error-block-node
Apr 4, 2023
Merged

Faulty code: improve error block node#13264
MarcusDenker merged 4 commits intopharo-project:Pharo12from
privat:faulty-code-error-block-node

Conversation

@privat
Copy link
Contributor

@privat privat commented Apr 3, 2023

RBErrorBlockNode are produce on block with a missing closing ].
Unfortunately, its contents did not include the temps possibly declared inside, and did have a bad stop value.

@privat
Copy link
Contributor Author

privat commented Apr 3, 2023

windows CI felt into a hole I presume

(self new
source: '[ | 1';
formattedCode: '[ | | 1';
formattedCode: '[ | | . 1';
Copy link
Member

Choose a reason for hiding this comment

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

There is a dot here because you consider that the end of a unclosed block is after its temporaries declaration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bad temps are stored into a synthetic first statement of the statement sequence to prevent ast visitors to mistake them for legit temp declarations. It's a ok design I think.

The formatter was not developed to format fauly ast (sometimes even non faulty ast :p ) so it just prints both bar (because temp declarations have two bars) and a dot (because statement, even synthetic , have a dot).

I have a wip branch that tries to fix theses formatting things.
But it's more cosmetic than useful, because formatting broken ast make little sense. The idea to check it in snippets is more to verify that parts are not lost, not the punctuation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And why there wasn't a dot before the PR. Because the contents of faulty blocks were the content of the statement sequence, not the statement sequence itself. Therefore, losing possible legitimate temps, and losing the information that the contents are statements. So the content were separated with simple space instead of dots.

@MarcusDenker
Copy link
Member

there is now a conflicet

@privat privat changed the base branch from Pharo12 to Pharo11 April 4, 2023 12:23
@privat privat changed the base branch from Pharo11 to Pharo12 April 4, 2023 12:23
@MarcusDenker MarcusDenker merged commit 58e2fed into pharo-project:Pharo12 Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants