Append newline to printed documents #391
Conversation
| return VisitAsync(node, context); | ||
| await VisitAsync(node, context).ConfigureAwait(false); | ||
| if (!context.NewLinePrinted && node is GraphQLDocument) | ||
| await writer.WriteLineAsync().ConfigureAwait(false); |
There was a problem hiding this comment.
Changes were made only within PrintAsync and only for printed documents, not for individual nodes being printed, or where VisitAsync was called directly and the context could be examined to determine whether or not a newline had been printed.
|
|
||
| extend type Foo @onType No newline at end of file | ||
| extend type Foo @onType |
There was a problem hiding this comment.
This PR fixes these issues -- generated documents did not include a terminating newline before.
There was a problem hiding this comment.
Hi, Shane! For the last year or two I completely abandoned GraphQL projects and GitHub from the moment of changing my work. Now I slowly read the accumulated mail and watch what has been done during this time. My progress is small and I'm not sure that someday I will catch everything completely. When dependabot has updated the version of the parser in one of my projects, I found that all my tests failed. It brought me here. Why do you generally call it an issue? I did not specifically add a new line at the end of the document. Are you faced with a specific problem like the impossibility of a correct search using grep? Honestly, I myself 50/50 about newline usage. I just want to feel good, knowing that there is a good reason why it is better to have newline than without it.
There was a problem hiding this comment.
Hard to explain; perhaps it’s personal preference. First it just aggravates me to see GitHub warning stating no newline at the end of the file. Second, in other file types, it’s much easier with a terminating newline to copy and paste code when you’re copying full lines that end with a newline. For example you can’t paste code at the end of a file if there’s no newline. Finally, the lack of a terminating newline (not specifically here) causes “problems” with my tests when I want to concatenate the output with additional data. So all of my tests concatenate a newline before ShouldMatchApproved, which this PR eliminated.
There was a problem hiding this comment.
Well, OK but I'm still 50/50 here. I updated my tests with trailing new line.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #391 +/- ##
==========================================
- Coverage 96.12% 96.10% -0.02%
==========================================
Files 89 89
Lines 5030 5032 +2
Branches 500 500
==========================================
+ Hits 4835 4836 +1
Misses 194 194
- Partials 1 2 +1 ☔ View full report in Codecov by Sentry. |
I'd like this merged in for v9.4.0 in preparation for GraphQL.NET v8.0. Note: GraphQL.NET v7.x uses the v8 parser.