Repository: LukasKalbertodt/bunt (language: rust)

Issue:
Feature request: Allow `tt` at format string position
Hi,

At the moment `bunt` [only allows literals at the format string position](https://docs.rs/bunt/0.2.1/src/bunt/lib.rs.html#216-223). `std:println`, however, [allows](https://doc.rust-lang.org/src/std/macros.rs.html#94-99) `tt`.

Unfortunately, this prevents `bunt` users from generating format strings via macros.

For example, I tried the following:

```rust
macro_rules! println_with_colored_prefix {
    ($color:literal, $id:expr, $fmt_str:literal $(, $arg:expr)*) => {
        bunt::println!(
            concat!("{$", $color, "}[{}][{}]{/$} ", $fmt_str),
            BIN.get().expect("Usage before static variable `BIN` set"),
            $id
            $(, $arg)*
        )
    };
}

macro_rules! say {
    ($($arg:expr),+) => {
        println_with_colored_prefix!("blue", $($arg),+)
    };
}

macro_rules! err {
    ($($arg:expr),+) => {
        println_with_colored_prefix!("red", $($arg),+)
    };
}

fn main() {
    let name = "foo";
    say!(name, "my message.");
    err!(name, "my error.");
}
```

Because `concat!("{$", $color, "}[{}][{}]{/$} ", $fmt_str)` isn't a literal, this code doesn't compile.

`std` allows this, however ([Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6a432fcffdcc41a3ef4fc4acc0305e7e):

```rust
macro_rules! my_println {
    ($fmt_str:literal $(, $arg:expr)*) => {
        println!(concat!("my prefix: ", $fmt_str), $(, $arg)*)
    };
}

fn main(){
    my_println!("foo")
}
```

From looking at the source code (see links above), it seems `bunt` would just have to replace `literal` in `$format_str:literal` with `tt` to support this.

Maybe the `tt` would have to be converted to a literal somehow, I'm not sure. The macros above are the only macros I have created so far :)



Discussion / hints:
For now, I use the following workaround:

```rust
macro_rules! say{
    ($id:expr, $fmt_str:literal $(, $arg:expr)*) => {{
        bunt::print!(
            "{$blue}[{}][{}]{/$} ",
            BIN.get().expect("Usage before static variable `BIN` set"),
            $id
        );
        bunt::println!($fmt_str $(, $arg)*)
    }};
}

macro_rules! err{
    ($id:expr, $fmt_str:literal $(, $arg:expr)*) => {{
        bunt::print!(
            "{$red}[{}][{}]{/$} ",
            BIN.get().expect("Usage before static variable `BIN` set"),
            $id
        );
        bunt::println!($fmt_str $(, $arg)*)
    }};
}
```

But I think `tt` in format string position would be a useful addition if this is easy to implement (probably also a bit more efficient. However, I'm not sure if it would be significant).
This is actually something I already thought about, precisely for the same reason: I have another wrapper macro that wants to pass a `concat!` invocation as format string. Unfortunately, this is tricky. 

> Maybe the tt would have to be converted to a literal somehow,

Exactly and that's the problem. `bunt` has to actually parse the string literal, i.e. needs access to the complete string literal data. Macros are lazily expanded by default. That means if you pass `concat!("a", "b")` to `bunt::println`, then `bunt` receives the tokens `concat`, `!`, `(`, `"a"`, `,`, `"b"` and `)` and NOT the result of `concat`. Of course `bunt` could try to interpret the macro invocation and concat the strings. But that only works for `concat!` and even that is fishy, if there is another `concat` macro in scope that works differently.

There is a relevant RFC that's not merged yet: https://github.com/rust-lang/rfcs/pull/2320
This would allow bunt to ask the compiler to expand that `concat` please. But yeah, that won't be stable anytime soon. 

But in the meantime, we could allow multiple string literals in that position. I.e. to allow `bunt::println!("foo" "bar" "{}x", 3)`. `bunt` could then easily concat all those literals. I think that would already be sufficient for many use cases. What do you think about that solution?
> Macros are lazily expanded by default.

That's unfortunate, eager expansion would make more sense, IMO (but there are most likely good reasons, why it was implemented this way initially).

> But in the meantime, we could allow multiple string literals in that position. I.e. to allow `bunt::println!("foo" "bar" "{}x", 3)`. bunt could then easily concat all those literals. I think that would already be sufficient for many use cases. What do you think about that solution?

Yes, that would be a good workaround, that probably covers most use cases.

What do you think about `bunt::println!(("foo", "bar", "{}x"), 3)`? 

With a match arm that matches a tuple in the first position, it should be easy to collect the values and pass them to `concat!`, and this variant has the advantage of being valid Rust (maybe that is better for syntax highlighting).

Fix patch:
diff --git a/examples/wrapper.rs b/examples/wrapper.rs
new file mode 100644
index 0000000..08a9a30
--- /dev/null
+++ b/examples/wrapper.rs
@@ -0,0 +1,47 @@
+//! Sometimes, you might want to wrap bunt's macros into your own
+//! project-specific macro. Often, you want to print a prefix or something like
+//! that.
+
+
+macro_rules! log {
+    ($fmt:literal $(, $arg:expr)* $(,)?) => {
+        bunt::println!(
+            // Bunt macros allow to pass an "array" of format strings which are
+            // then concatenated by bunt.
+            [
+                // Our prefix
+                "[{[magenta] log_module_path}] ",
+                // What the user passed
+                $fmt,
+                // Our postfix
+                " {$cyan}({log_file}:{log_line}){/$}",
+            ],
+            $($arg ,)*
+            log_module_path = std::module_path!(),
+            log_file = std::file!(),
+            log_line = std::line!(),
+        )
+
+        // This solution is not optimal though. For one, it would be nice to
+        // `stringify!(std::module_path!())` instead of passing it as runtime
+        // string argument. That's not possible because macro expansions are
+        // lazy.
+        //
+        // Futhermore, we pass the arguments with names like `log_module_path`.
+        // In theory, the user could also use a named argument with that name.
+        // This `log!` macro is very much an internal helper macro and not
+        // something you would want to put into your public API.
+    };
+}
+
+
+fn main() {
+    log!("Hello {}", "peter");
+    banana::do_something();
+}
+
+mod banana {
+    pub fn do_something() {
+        log!("I'm doing something with {[yellow]:?}", vec![1, 2, 4]);
+    }
+}
diff --git a/macros/src/parse/fmt.rs b/macros/src/parse/fmt.rs
index c04ff4a..fe54a8f 100644
--- a/macros/src/parse/fmt.rs
+++ b/macros/src/parse/fmt.rs
@@ -1,6 +1,6 @@
 use proc_macro2::{
     Span,
-    token_stream::IntoIter as TokenIterator,
+    token_stream::IntoIter as TokenIterator, TokenTree, Delimiter,
 };
 use unicode_xid::UnicodeXID;
 use std::str::Chars;
@@ -15,6 +15,7 @@ use super::{parse, expect_helper_group, lit::expect_str_literal};
 
 
 impl FormatStr {
+    /// Parses `["foo"]`, `["foo" "bar"]`.
     pub(crate) fn parse(it: &mut TokenIterator) -> Result<Self, Error> {
         /// Searches for the next closing `}`. Returns a pair of strings, the
         /// first starting like `s` and ending at the closing brace, the second
@@ -29,8 +30,44 @@ impl FormatStr {
             Ok((&s[..end], &s[end + 1..]))
         }
 
-        let (inner, _) = expect_helper_group(it.next())?;
-        let (raw, span) = parse(inner, expect_str_literal)?;
+        // We expect a []-delimited group
+        let (inner, span) = match it.next() {
+            Some(TokenTree::Group(g)) if g.delimiter() == Delimiter::Bracket => {
+                (g.stream(), g.span())
+            }
+            Some(TokenTree::Group(g)) => {
+                return Err(err!(
+                    g.span(),
+                    "expected `[]` delimited group, but delimiter is {:?} (note: do not use \
+                        the macros from `bunt-macros` directly, but only through `bunt`)",
+                    g.delimiter(),
+                ));
+            }
+            Some(tt) => {
+                return Err(err!(
+                    tt.span(),
+                    "expected `[]` delimited group, but found different token tree (note: do \
+                        not use the macros from `bunt-macros` directly, but only through `bunt`)",
+                ))
+            }
+            None => return Err(err!("expected `[]` delimited group, found EOF")),
+        };
+
+        if inner.is_empty() {
+            return Err(err!(
+                span,
+                "at least one format string has to be provided, but `[]` was passed (note: do not \
+                    use the macros from `bunt-macros` directly, but only through `bunt`)"
+            ));
+        }
+
+        // Concat all string literals
+        let mut raw = String::new();
+        for tt in inner {
+            let (literal, _) = expect_helper_group(Some(tt))?;
+            let (string_data, _) = parse(literal, expect_str_literal)?;
+            raw += &string_data;
+        }
 
         // Scan the whole string
         let mut fragments = Vec::new();
diff --git a/src/lib.rs b/src/lib.rs
index 19b1905..43f4b14 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -116,6 +116,29 @@
 //! want to give your users the choice of color usage, e.g. via a `--color` CLI
 //! argument.
 //!
+//!
+//! # Passing multiple format strings (`concat!` replacement)
+//!
+//! In many cases, users wish to call `concat!` and pass the result as format
+//! string to bunt's macros, e.g. `bunt::println!(concat!("foo", "bar"))`. This
+//! is mainly used if you want to write your own macro to wrap bunt's macros.
+//! Unfortunately, this is not easily possible as macros are expaned lazily. See
+//! [issue #15](https://github.com/LukasKalbertodt/bunt/issues/15) for more
+//! information.
+//!
+//! As a workaround for this fairly common use case, bunt allows passing an
+//! "array of format strings", like so:
+//!
+//! ```
+//! bunt::println!(["foo ", "{[red]} bar"], 27);
+//! ```
+//!
+//! All given strings will be concatenated by `bunt`. So the above code is
+//! equivalent to `bunt::println!("foo {[red]} bar", 27)`.
+//!
+//! For most users this feature is irrelevant. If possible, pass the format
+//! string as single string literal.
+//!
 
 #![deny(intra_doc_link_resolution_failure)]
 
@@ -152,8 +175,11 @@ pub extern crate bunt_macros;
 #[macro_export]
 macro_rules! write {
     ($target:expr, $format_str:literal $(, $arg:expr)* $(,)?) => {
+        $crate::write!($target, [$format_str] $(, $arg )*)
+    };
+    ($target:expr, [$($format_str:literal),+ $(,)?] $(, $arg:expr)* $(,)?) => {
         $crate::bunt_macros::write!(
-            $target $format_str $( $arg )*
+            $target [$($format_str)+] $( $arg )*
         )
     };
 }
@@ -175,8 +201,11 @@ macro_rules! write {
 #[macro_export]
 macro_rules! writeln {
     ($target:expr, $format_str:literal $(, $arg:expr)* $(,)?) => {
+        $crate::writeln!($target, [$format_str] $(, $arg )*)
+    };
+    ($target:expr, [$($format_str:literal),+ $(,)?] $(, $arg:expr)* $(,)?) => {
         $crate::bunt_macros::writeln!(
-            $target $format_str $( $arg )*
+            $target [$($format_str)+] $( $arg )*
         )
     };
 }
@@ -196,9 +225,12 @@ macro_rules! writeln {
 #[macro_export]
 macro_rules! print {
     ($format_str:literal $(, $arg:expr)* $(,)?) => {
+        $crate::print!([$format_str] $(, $arg )*)
+    };
+    ([$($format_str:literal),+ $(,)?] $(, $arg:expr)* $(,)?) => {
         $crate::bunt_macros::write!(
             ($crate::termcolor::StandardStream::stdout($crate::termcolor::ColorChoice::Auto))
-            $format_str $( $arg )*
+            [$($format_str)+] $( $arg )*
         ).expect("failed to write to stdout in `bunt::print`")
     };
 }
@@ -215,9 +247,12 @@ macro_rules! print {
 #[macro_export]
 macro_rules! println {
     ($format_str:literal $(, $arg:expr)* $(,)?) => {
+        $crate::println!([$format_str] $(, $arg )*)
+    };
+    ([$($format_str:literal),+ $(,)?] $(, $arg:expr)* $(,)?) => {
         $crate::bunt_macros::writeln!(
             ($crate::termcolor::StandardStream::stdout($crate::termcolor::ColorChoice::Auto))
-            $format_str $( $arg )*
+            [$($format_str)+] $( $arg )*
         ).expect("failed to write to stdout in `bunt::println`")
     };
 }
@@ -237,9 +272,12 @@ macro_rules! println {
 #[macro_export]
 macro_rules! eprint {
     ($format_str:literal $(, $arg:expr)* $(,)?) => {
+        $crate::eprint!([$format_str] $(, $arg )*)
+    };
+    ([$($format_str:literal),+ $(,)?] $(, $arg:expr)* $(,)?) => {
         $crate::bunt_macros::write!(
             ($crate::termcolor::StandardStream::stderr($crate::termcolor::ColorChoice::Auto))
-            $format_str $( $arg )*
+            [$($format_str)+] $( $arg )*
         ).expect("failed to write to stderr in `bunt::eprint`")
     };
 }
@@ -256,9 +294,12 @@ macro_rules! eprint {
 #[macro_export]
 macro_rules! eprintln {
     ($format_str:literal $(, $arg:expr)* $(,)?) => {
+        $crate::eprintln!([$format_str] $(, $arg )*)
+    };
+    ([$($format_str:literal),+ $(,)?] $(, $arg:expr)* $(,)?) => {
         $crate::bunt_macros::writeln!(
             ($crate::termcolor::StandardStream::stderr($crate::termcolor::ColorChoice::Auto))
-            $format_str $( $arg )*
+            [$($format_str)+] $( $arg )*
         ).expect("failed to write to stderr in `bunt::eprintln`")
     };
 }


Test patch:
diff --git a/tests/main.rs b/tests/main.rs
index bb6ebf0..4551df1 100644
--- a/tests/main.rs
+++ b/tests/main.rs
@@ -1,6 +1,6 @@
 use std::fmt;
 use bunt::{
-    write, writeln, print, println,
+    write, writeln, print, println, eprint, eprintln,
     termcolor::Buffer,
 };
 
@@ -305,3 +305,29 @@ fn set_color_error() {
     assert!(res.is_err());
     assert_eq!(res.unwrap_err().kind(), io::ErrorKind::NotFound);
 }
+
+#[test]
+fn concat_fmt_strings() {
+    check!("hello" == ["hello"]);
+    check!("hello" == ["hello",]);
+    check!("foobar" == ["foo", "bar"]);
+    check!("foobar" == ["foo", "bar",]);
+    check!("foo 27 bar" == ["foo ", "{} bar"], 27);
+    check!("abtruecd" == ["a", "b", "{}", "c", "d"], true);
+
+    check!(
+        "a\nb\tc\rd\0e\x48f\u{50}g\u{228}h\u{fffe}i\u{1F923}j" ==
+        ["a\n", "b\tc\r", "d\0e\x48f\u{50}", "g\u{228}h\u{fffe}i", "\u{1F923}j"]
+    );
+}
+
+#[test]
+fn concat_fmt_strings_all_strings() {
+    let mut b = buf();
+    let _ = write!(b, ["a", "{} b"], 27);
+    let _ = writeln!(b, ["a", "{} b"], 27);
+    print!(["a", "{} \r"], 27);
+    eprint!(["a", "{} \r"], 27);
+    println!(["", "{}"], "");
+    eprintln!(["", "{}"], "");
+}