Skip to content

Conversation

@sungam3r
Copy link
Member

No description provided.

@sungam3r sungam3r requested a review from Shane32 October 16, 2023 07:15
@sungam3r sungam3r self-assigned this Oct 16, 2023
@sungam3r sungam3r added the enhancement New feature or request label Oct 16, 2023
@github-actions github-actions bot added the test Pull request that adds new or changes existing tests label Oct 16, 2023
var ast = text.Parse<GraphQLDirectives>().ShouldNotBeNull();
ast.Items.Count.ShouldBe(2);

// TODO: fix leading space before first directive
Copy link
Member Author

Choose a reason for hiding this comment

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

@my @your instead of @my @your now. It could be fixes as well checking parent node but for directives parent is always not null - it is Directives container, so actually grand parent is needed. Moreover, even after such check some tests failed (see Printer_Should_Print_Pretty_If_Directives_Skipped). The core problem is that some arbitrary nodes can be skipped so it's not obvious how to handle whitespaces at all. General approach may be to require each node to print their leading whitespace if it should do that. Downside - it will complicate all printing code. I do not want to do that just to handle very rare cases.

@sungam3r sungam3r merged commit 651da05 into master Oct 16, 2023
@sungam3r sungam3r deleted the fixes branch October 16, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request test Pull request that adds new or changes existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants