Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Term::deep_repr by the pretty printer #1262

Merged
merged 2 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/bin/nickel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use nickel_lang::program::Program;
use nickel_lang::repl::query_print;
#[cfg(feature = "repl")]
use nickel_lang::repl::rustyline_frontend;
use nickel_lang::term::{RichTerm, Term};
use nickel_lang::term::RichTerm;
use nickel_lang::{serialize, serialize::ExportFormat};
use std::path::{Path, PathBuf};
use std::{
Expand Down Expand Up @@ -207,9 +207,7 @@ fn main() {
stdout,
format,
}) => export_doc(&mut program, opts.file.as_ref(), output, stdout, format),
None => program
.eval_full()
.map(|t| println!("{}", Term::from(t).deep_repr())),
None => program.eval_full().map(|t| println!("{t}")),
};

if let Err(err) = result {
Expand Down
19 changes: 15 additions & 4 deletions src/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,17 @@ where
}

impl<'a, D, A> Pretty<'a, D, A> for RichTerm
where
D: NickelAllocatorExt<'a, A>,
D::Doc: Clone,
A: Clone + 'a,
{
fn pretty(self, allocator: &'a D) -> DocBuilder<'a, D, A> {
self.as_ref().pretty(allocator)
}
}

impl<'a, D, A> Pretty<'a, D, A> for &Term
where
D: NickelAllocatorExt<'a, A>,
D::Doc: Clone,
Expand All @@ -418,7 +429,7 @@ where
fn pretty(self, allocator: &'a D) -> DocBuilder<'a, D, A> {
use Term::*;

match self.as_ref() {
match self {
Null => allocator.text("null"),
Bool(v) => allocator.as_string(v),
Num(n) => allocator.as_string(format!("{}", n.to_sci())),
Expand Down Expand Up @@ -453,16 +464,16 @@ where
// TODO Pattern destructuring to implement.
FunPattern(..) => {
let mut params = vec![];
let mut rt = &self;
while let FunPattern(id, dst, t) = rt.as_ref() {
let mut rt = self;
while let FunPattern(id, dst, t) = rt {
params.push(if let Some(id) = id {
allocator
.as_string(id)
.append(allocator.text("@").append(dst.pretty(allocator)))
} else {
dst.pretty(allocator)
});
rt = t;
rt = t.as_ref();
}
allocator
.text("fun")
Expand Down
4 changes: 2 additions & 2 deletions src/repl/rustyline_frontend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn repl(histfile: PathBuf, color_opt: ColorOpt) -> Result<(), InitError> {
}),
Ok(Command::Print(exp)) => {
match repl.eval_full(&exp) {
Ok(EvalResult::Evaluated(rt)) => println!("{}\n", rt.as_ref().deep_repr()),
Ok(EvalResult::Evaluated(rt)) => println!("{rt}"),
Ok(EvalResult::Bound(_)) => (),
Err(err) => program::report(repl.cache_mut(), err, color_opt),
};
Expand All @@ -120,7 +120,7 @@ pub fn repl(histfile: PathBuf, color_opt: ColorOpt) -> Result<(), InitError> {
}
Ok(line) => {
match repl.eval_full(&line) {
Ok(EvalResult::Evaluated(rt)) => println!("{}\n", rt.as_ref().deep_repr()),
Ok(EvalResult::Evaluated(rt)) => println!("{rt}\n"),
Ok(EvalResult::Bound(_)) => (),
Err(err) => program::report(repl.cache_mut(), err, color_opt),
};
Expand Down
6 changes: 2 additions & 4 deletions src/repl/simple_frontend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub fn input<R: Repl>(repl: &mut R, line: &str) -> Result<InputResult, InputErro
Ok(Command::Print(exp)) => repl
.eval_full(&exp)
.map(|res| match res {
EvalResult::Evaluated(rt) => InputResult::Success(rt.as_ref().deep_repr()),
EvalResult::Evaluated(rt) => InputResult::Success(format!("{rt}\n")),
EvalResult::Bound(_) => InputResult::Blank,
})
.map_err(InputError::from),
Expand All @@ -82,9 +82,7 @@ pub fn input<R: Repl>(repl: &mut R, line: &str) -> Result<InputResult, InputErro
} else {
repl.eval_full(line)
.map(|eval_res| match eval_res {
EvalResult::Evaluated(rt) => {
InputResult::Success(format!("{}\n", rt.as_ref().deep_repr()))
}
EvalResult::Evaluated(rt) => InputResult::Success(format!("{rt}\n")),
EvalResult::Bound(_) => InputResult::Success(String::new()),
})
.map_err(InputError::from)
Expand Down
37 changes: 1 addition & 36 deletions src/term/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,40 +742,6 @@ impl Term {
}
}

/// Return a deep string representation of a term, used for printing in the REPL
pub fn deep_repr(&self) -> String {
match self {
Term::Record(r) | Term::RecRecord(r, ..) => {
let fields_str: Vec<String> = r
.fields
.iter()
.map(|(ident, field)| {
if let Some(ref value) = field.value {
format!("{} = {}", ident, value.as_ref().deep_repr())
} else {
format!("{ident}")
}
})
.collect();

let suffix = match self {
Term::RecRecord(_, dyn_fields, ..) if !dyn_fields.is_empty() => ", ..",
_ => "",
};

format!("{{ {}{} }}", fields_str.join(", "), suffix)
}
Term::Array(elements, _) => {
let elements_str: Vec<String> = elements
.iter()
.map(|term| term.as_ref().deep_repr())
.collect();
format!("[ {} ]", elements_str.join(", "))
}
_ => self.shallow_repr(),
}
}

/// Determine if a term is in evaluated from, called weak head normal form (WHNF).
pub fn is_whnf(&self) -> bool {
match self {
Expand Down Expand Up @@ -1734,9 +1700,8 @@ impl From<Term> for RichTerm {
impl std::fmt::Display for RichTerm {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
use crate::pretty::*;
use pretty::BoxAllocator;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to move this specific use from the local function to the top-level? We don't really have consistent rules right now about where to place use statements, but I vaguely intuit that it makes more sense to keep it local if it's very specific to the current function, and doesn't have much to do with the rest of the module, which seems to be the case here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a very good question. It might have happened during a previous iteration, but right now there's no good reason. I think I'll revert this part.


let allocator = BoxAllocator;
let allocator = pretty::BoxAllocator;

let doc: DocBuilder<_, ()> = self.clone().pretty(&allocator);
doc.render_fmt(80, f)
Expand Down
3 changes: 3 additions & 0 deletions tests/snapshot/inputs/eval/escaping.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# capture = 'stdout'
# command = []
"a\"bcd\""
3 changes: 3 additions & 0 deletions tests/snapshot/inputs/eval/records.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# capture = 'stdout'
# command = []
{ a = true, b | Number = 6 * 7 }
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: tests/snapshot/main.rs
expression: out
---
"a\"bcd\""

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: tests/snapshot/main.rs
expression: out
---
{ a = true, b | Number = 42, }