Skip to content

Commit

Permalink
fix: blank lines between things with docstrings (#954)
Browse files Browse the repository at this point in the history
The cynic-parser pretty printing wasn't putting blank lines before
fields & arguments that had docstrings, which made the output quite
visually noisy. This implements logic to do this, resulting in much
nicer output for this case.
  • Loading branch information
obmarg authored May 18, 2024
1 parent de7d816 commit e952085
Show file tree
Hide file tree
Showing 5 changed files with 8,431 additions and 34 deletions.
58 changes: 27 additions & 31 deletions cynic-parser/src/printing/type_system.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
mod argument_sequence;
mod field_sequence;

use pretty::{DocAllocator, Pretty};

use crate::type_system::*;

use self::{argument_sequence::ArgumentSequence, field_sequence::FieldSequence};

use super::escape_string;

type Allocator<'a> = pretty::Arena<'a>;
Expand Down Expand Up @@ -181,17 +186,15 @@ impl<'a> Pretty<'a, Allocator<'a>> for NodeDisplay<ObjectDefinition<'a>> {
);
}

let mut fields = self.0.fields().peekable();
if fields.peek().is_some() {
let fields = self.0.fields();
if fields.len() != 0 {
builder = builder
.append(allocator.space())
.append(allocator.text("{"))
.append(
allocator
.hardline()
.append(
allocator.intersperse(fields.map(NodeDisplay), allocator.hardline()),
)
.append(FieldSequence::new(fields))
.nest(2),
)
.append(allocator.hardline())
Expand All @@ -217,14 +220,12 @@ impl<'a> Pretty<'a, Allocator<'a>> for NodeDisplay<FieldDefinition<'a>> {

let mut arguments_pretty = allocator.nil();

let mut arguments = self.0.arguments().peekable();
let arguments = self.0.arguments();

if arguments.peek().is_some() {
if arguments.len() != 0 {
arguments_pretty = allocator
.line_()
.append(
allocator.intersperse(arguments.map(NodeDisplay), comma_or_newline(allocator)),
)
.append(ArgumentSequence::new(arguments))
.nest(2)
.append(allocator.line_())
.parens()
Expand Down Expand Up @@ -283,17 +284,16 @@ impl<'a> Pretty<'a, Allocator<'a>> for NodeDisplay<InterfaceDefinition<'a>> {
);
}

let mut fields = self.0.fields().peekable();
if fields.peek().is_some() {
let fields = self.0.fields();
if fields.len() != 0 {
builder = builder
.append(allocator.space())
.append(allocator.text("{"))
.append(allocator.hardline())
.append(allocator.text(" "))
.append(
allocator
.intersperse(fields.map(NodeDisplay), allocator.hardline())
.align(),
.hardline()
.append(FieldSequence::new(fields))
.nest(2),
)
.append(allocator.hardline())
.append(allocator.text("}"));
Expand Down Expand Up @@ -369,17 +369,17 @@ impl<'a> Pretty<'a, Allocator<'a>> for NodeDisplay<EnumDefinition<'a>> {
);
}

let values = self.0.values().collect::<Vec<_>>();
let values = self.0.values();

if !values.is_empty() {
if values.len() != 0 {
builder = builder
.append(allocator.space())
.append(allocator.text("{"))
.append(allocator.hardline())
.append(
allocator
.intersperse(values.into_iter().map(NodeDisplay), allocator.hardline())
.indent(2),
.hardline()
.append(FieldSequence::new(values))
.nest(2),
)
.append(allocator.hardline())
.append(allocator.text("}"));
Expand Down Expand Up @@ -438,17 +438,15 @@ impl<'a> Pretty<'a, Allocator<'a>> for NodeDisplay<InputObjectDefinition<'a>> {
);
}

let mut fields = self.0.fields().peekable();
if fields.peek().is_some() {
let fields = self.0.fields();
if fields.len() != 0 {
builder = builder
.append(allocator.space())
.append(allocator.text("{"))
.append(
allocator
.hardline()
.append(
allocator.intersperse(fields.map(NodeDisplay), allocator.hardline()),
)
.append(FieldSequence::new(fields))
.nest(2),
)
.append(allocator.hardline())
Expand Down Expand Up @@ -515,14 +513,12 @@ impl<'a> Pretty<'a, Allocator<'a>> for NodeDisplay<DirectiveDefinition<'a>> {
.append("@")
.append(self.0.name());

let mut arguments = self.0.arguments().peekable();
let arguments = self.0.arguments();

if arguments.peek().is_some() {
if arguments.len() != 0 {
let arguments = allocator
.line_()
.append(
allocator.intersperse(arguments.map(NodeDisplay), comma_or_newline(allocator)),
)
.append(ArgumentSequence::new(arguments))
.nest(2)
.append(allocator.line_())
.parens()
Expand Down
46 changes: 46 additions & 0 deletions cynic-parser/src/printing/type_system/argument_sequence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
use std::iter::Enumerate;

use pretty::{DocAllocator, Pretty};

use super::{comma_or_newline, Allocator, InputValueDefinition, NodeDisplay};

/// A sequence of things with docstrings attached.
///
/// This will print each entry with a prefix of:
/// - first entry gets no prefix
/// - when no docstring: just a single hardline prefix
/// - when a docstring: two hardline prefix
///
/// This is used for displaying fields (both input fields & output fields)
/// but arguments should use ArgumentSequence
pub struct ArgumentSequence<'a> {
iterator: Enumerate<crate::type_system::iter::Iter<'a, InputValueDefinition<'a>>>,
}

impl<'a> ArgumentSequence<'a> {
pub fn new(iterator: crate::type_system::iter::Iter<'a, InputValueDefinition<'a>>) -> Self {
ArgumentSequence {
iterator: iterator.enumerate(),
}
}
}

impl<'a> Pretty<'a, Allocator<'a>> for ArgumentSequence<'a> {
fn pretty(self, allocator: &'a Allocator<'a>) -> pretty::DocBuilder<'a, Allocator<'a>, ()> {
let mut document = allocator.nil();
for (index, item) in self.iterator {
if index != 0 {
if item.description().is_some() {
// We use a hardcoded `\n` for the first newline here because
// pretty always adds an indent on line but we want this line blank
document = document.append(allocator.text("\n").flat_alt(allocator.text(", ")));
document = document.append(allocator.hardline());
} else {
document = document.append(allocator.line().flat_alt(allocator.text(", ")));
}
}
document = document.append(NodeDisplay(item));
}
document
}
}
88 changes: 88 additions & 0 deletions cynic-parser/src/printing/type_system/field_sequence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
use std::iter::Enumerate;

use pretty::{DocAllocator, Pretty};

use crate::common::IdOperations;

use super::{
iter::IdReader, Allocator, EnumValueDefinition, FieldDefinition, InputValueDefinition,
NodeDisplay, TypeSystemId,
};

/// A sequence of things with docstrings attached.
///
/// This will print each entry with a prefix of:
/// - first entry gets no prefix
/// - when no docstring: just a single hardline prefix
/// - when a docstring: two hardline prefix
///
/// This is used for displaying fields (both input fields & output fields)
/// but arguments should use ArgumentSequence
pub struct FieldSequence<'a, T>
where
T: IdReader,
{
iterator: Enumerate<crate::type_system::iter::Iter<'a, T>>,
}

impl<'a, T> FieldSequence<'a, T>
where
T: IdReader,
T::Id: IdOperations,
{
pub fn new(iterator: crate::type_system::iter::Iter<'a, T>) -> Self {
FieldSequence {
iterator: iterator.enumerate(),
}
}
}

trait DocstringedItem {
fn has_docstring(&self) -> bool;
}

impl DocstringedItem for FieldDefinition<'_> {
fn has_docstring(&self) -> bool {
self.description().is_some()
}
}

impl DocstringedItem for InputValueDefinition<'_> {
fn has_docstring(&self) -> bool {
self.description().is_some()
}
}

impl DocstringedItem for EnumValueDefinition<'_> {
fn has_docstring(&self) -> bool {
self.description().is_some()
}
}

impl<'a, T> Pretty<'a, Allocator<'a>> for FieldSequence<'a, T>
where
T: DocstringedItem + IdReader,
T::Id: IdOperations,
// This is a bit much :/
<<T as IdReader>::Id as TypeSystemId>::Reader<'a>: DocstringedItem,
NodeDisplay<<<T as IdReader>::Id as TypeSystemId>::Reader<'a>>: Pretty<'a, Allocator<'a>>,
{
fn pretty(self, allocator: &'a Allocator<'a>) -> pretty::DocBuilder<'a, Allocator<'a>, ()> {
let mut document = allocator.nil();
for (index, item) in self.iterator {
if index != 0 {
if item.has_docstring() {
// We use a hardcoded `\n` for the first newline here because
// pretty always adds an indent but we want this line blank
document = document
.append(allocator.text("\n"))
.append(allocator.hardline());
} else {
document = document.append(allocator.hardline());
}
}
document = document.append(NodeDisplay(item));
}
document
}
}
Loading

0 comments on commit e952085

Please sign in to comment.