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

Rename print!()/println!() to printf!()/printlnf!() #7779

Merged
merged 1 commit into from
Jul 14, 2013

Conversation

lilyball
Copy link
Contributor

The new names make it obvious that these generate formatted output.

Add a one-argument case that uses %? to format, just like the other
format-using macros (e.g. info!()).

@alexcrichton
Copy link
Member

I'd actually vote for removing printlnf altogether. Everyone's familiar with printf and how it doesn't have a trailing newline, and printlnf just kinda sounds weird...

@lilyball
Copy link
Contributor Author

@alexcrichton: The benefit of keeping printlnf!() is I can say

printlnf!(myvec);

If we didn't support the one-arg case then I'd agree with you.

@pcwalton
Copy link
Contributor

Isn't it usually printfln (following D)?

The new names make it obvious that these generate formatted output.

Add a one-argument case that uses %? to format, just like the other
format-using macros (e.g. info!()).
@metajack
Copy link
Contributor

printf screams C to me. I think print! and println! are fine names.

In fact, I think the common case will be the one that adds a newline, so really it should have the shorter name. I suggest println! becomes print!and print! becomes something else; maybe printp! (for partial) or maybe prints! (for string).

@lilyball
Copy link
Contributor Author

@metajack: We had a nice long discussion in IRC about this already. Some people feel that if we have println!() then this will become the de-facto way to print and println() will be an under-the-hood function, but having formatted output be the de-facto way to print is not really a good idea.

@alexcrichton
Copy link
Member

@kballard oh good point, although I'd be with @pcwalton to go with printfln instead of printlnf

@lilyball
Copy link
Contributor Author

A few minutes ago I pushed @pcwalton's suggested change, if anyone wants to r+ this.

@metajack
Copy link
Contributor

@kballard I really don't think you can stop people from using some formatted print function to print by default. It's going to be the most general and convenient way to print stuff, and so people will always reach for it first.

I assume the worry is that people pass arbitrary strings as the format string. Can't the macro-ness of it also make it safe? Outlawing anything but string literals would seem the easiest way. People can always fall back to fmt! and println if they want to generate format strings programmatically and then pass them.

@lilyball
Copy link
Contributor Author

@metajack: You cannot pass user input as the format string. This uses fmt!() internally, which evaluates the format string at compile-time and verifies that the number and types of the arguments match up with the format tokens.

@metajack
Copy link
Contributor

@kballard if safety is not a concern, why would we care if users reached for println! instead of println. Perhaps someone could summarize the IRC discussion, as I'm missing the rationale for more complicated names.

@lilyball
Copy link
Contributor Author

@metajack: This has to do with how you handle the single-argument case. If everyone used println! instead of println then a single string literal would need to be printed directly. Which means you wouldn't be able to pass any other argument, including a variable, which means the way to print a string variable looks like println!("%s", str). This is a needlessly complicated way of saying println(str).

In general, it's more useful to interpret the single-arg case as allowing any type of arg (both non-strings and non-literals), and printing it using the %? token. This lets me say printfln!(myvec). This is also consistent with the way other format-using macros, such as info!(), behave.

@metajack
Copy link
Contributor

Maybe we're going about this all wrong. Probably what we want is not formatted printing but var args println that converts all its arguments and separates them with spaces. This is what println and print in Clojure or Python do for example.

Sometimes format strings are useful, but the normal case of printing out various things for debugging is not one of them. So instead of

println!("My name is %s", name);

we should just have:

println!("My name is", name);

This would also work for:

println!("name=", name, "age=", age, foobar);

If you need something fancier, you use fmt!, but i suspect it would get used like:

println!("your balance is", fmt!("%0.2f", balance));

@lilyball
Copy link
Contributor Author

@metajack: Well, the goal of the macro is to replace the exceedingly common pattern println(fmt!(...)). Honestly, if it were up to me, I wouldn't bother with the macro in the first place, because I don't like having multiple ways to do the same thing.

@bstrie
Copy link
Contributor

bstrie commented Jul 14, 2013

Pull requests are not the place to discuss design. Let's bring this up on the mailing list instead. Honestly I'm not even sure these macros are warranted in the first place.

@metajack
Copy link
Contributor

@bstrie You're right. I started a thread on the list.

bors added a commit that referenced this pull request Jul 14, 2013
The new names make it obvious that these generate formatted output.

Add a one-argument case that uses %? to format, just like the other
format-using macros (e.g. info!()).
@bors bors closed this Jul 14, 2013
@bors bors merged commit 3b02589 into rust-lang:master Jul 14, 2013
@bstrie
Copy link
Contributor

bstrie commented Jul 14, 2013

Also I apologize for being snippy, was a bit tired yesterday. :)

@lilyball lilyball deleted the print-macro-args branch July 14, 2013 21:34
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 21, 2021
make test module detection more strict

I started with some small improvements to clippy_utils/src/lib.rs, but then found that our "test" module detection would also catch words containing "test" like e.g. "attestation". So I made this a bit more strict (splitting by `'_'` and checking for `test` or `tests`), adding a test case as I went.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants