Use more descriptive names for pp / epp

[pablosichert]

This is just a nit-pick – but I think it would be nice to use more descriptive identifiers for pp/epp, e.g. pretty_print:

grmtools/lrpar/src/lib/parser.rs

Lines 617 to 621 in 439201b

	pub fn pp<'a>(
	&self,
	lexer: &dyn Lexer<StorageT>,
	epp: &dyn Fn(TIdx<StorageT>) -> Option<&'a str>
	) -> String {

Those functions might not be needed at all if you implement std::fmt::Display / std::fmt::Debug, but I haven't looked into this case here in detail yet.

[ltratt]

Most of the pp functions can't be Display/Debug because they require a reference to something (generally the grammar) in order to display something useful.

pp doesn't worry me as a name too much, unless @ptersilie also thinks it would be better expanded.

%epp is subtly different, in that it has the virtue that it's not obvious to anyone what it means -- I believe that something adequately descriptive would be hard to come up with and quite long! So keeping it short at least means that people need to look it up. I know that sounds like an odd justification, but it's an odd feature!

[pablosichert]

Makes sense (even though I personally wouldn't use such short acronyms in a public API). I was just curious about the motivation behind this.

[pablosichert]

I had a random thought on this one: You can actually implement fmt::Display for tuples, so you wouldn't even need to choose a name. Not sure how discoverable that would be, though.

Edit: I was mistaken and that was not true as of the time of my writing.

[ltratt]

I might be missing something, but I'm not sure what tuples have to do with this?

[pablosichert]

My idea was to implement

impl<'a> std::fmt::Display for (&dyn Lexer<StorageT>, &dyn Fn(TIdx<StorageT>) -> Option<&'a str>) {
    ...
}

However, I just figured out that that's also not possible at the moment:

struct Foo {}

struct Bar {}

impl std::fmt::Display for (Foo, Bar) {}

 --> src/main.rs:5:1
  |
5 | impl std::fmt::Display for (Foo, Bar) {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate
  |
  = note: the impl does not reference only types defined in this crate
  = note: define and implement a trait or new type instead

Because tuples are currently not marked #[fundamental]: rust-lang/rust#31682.

[ltratt]

Because tuples are currently not marked #[fundamental]: rust-lang/rust#31682.

I did not know that!