Make Display impl respect formatting parameters

[martin-t]

Fixes #57

I don't think there's a cleaner (without another macro) way to handle the repetition.

I used fmt_prefix but i noticed you prefer naming without underscores, should i rename it to fmtprefix?

Not sure how to proceed with the doc comments:

Option 1: keep it as it: e.g. "Displays the vector, formatted as ({}, {}, {})." - it wouldn't be completely right since it now passes the formatting params correctly. Which actually reminds me that this PR is technically a breaking change.

Option 2: e.g. "Displays the vector, formatted as ({...}, {...}, {...}) where ... are the actual formatting parameters." - would pass this into the macro as $fmt in addition to $fmt_prefix.

Option 3: make it somewhat vague like "Displays the vector, numbers are separated by commas." - would also need a special case for the vectors with prefixes.

Thoughts?

Also when we agree how to do this, I should do the same for matrices.

[yoanlcq]

That looks very good to me!

For the doc comments, I would say option 2 is the best looking one.

fmt_prefix is fine to me, I don't actually have a strong opinion on the matter when it's somewhat "internal".

You're right about this being technically a breaking change, but I think it's really fine, for multiple reasons (it looks like I'm arguing, but it's really more about keeping them for reference just in case):

• I think (but I may misunderstand) the point of Display is to print some object in a human-readable form, and that form may be subjective, and therefore subject to changes (unless the object is "built-in", like a tuple of primitives or something).
  I mean at the very least, I wouldn't expect anyone to try to parse the result of Display (even like, saving it to a file for reading later). Users who would do that should actually convert the vector into a stable type, like a tuple or array, which Display implementation has certainly been set in stone a while ago. Or better, I would expect them to have their own reliable serialization/deserialization routines, or use serde which works very well.
• vek's major version is still 0. We can add breaking changes and increment the minor version.
• Even assuming someone has code that parses the result of Display of a vector (hopefully unlikely!), and that someone updates to the newer version with this fix, then how likely is it that the fix would in fact "break" their code? In the very, very unlikely event that this broke their code, then they would probably figure out the problem easily and move on, or so I hope.

[yoanlcq]

By the way, feel free to add yourself to the list of contributors in the README.md and Cargo.toml files, if you want. :)

[martin-t]

I did the same for matrices and realized i don't need macro recursion so i also replaced the recursive solution with iteration on vectors because it looks cleaner to me. Didn't bench perf - not sure if it matters for printing. Relatedly - matrices have two Display impls - for row-major and column-major - if perf doesn't matter, could be simplified into one by using Index.

This PR solved the original issue i had but i am not sure all its effects are so desirable - e.g.:

let v1 = Vec2::new(5.322, 6.464616);
let v2 = Vec2::new(-5.0003, -0.0);
println!("{1:6.2} | {0:6.2}", v1, v2);

aligns the individual elements which might be surprising if people want to align the whole result instead. (Sry no example in comment, GitHub removes consecutive spaces fsr) Not sure what other similar crates do - your thoughts?

[yoanlcq]

I think performance will be just fine, I would expect the bottleneck to be the actual file/console I/O that takes place.

[...] aligns the individual elements which might be surprising if people want to align the whole result instead

I think this is very situational. In the extremely rare (I suppose) case where people do want this, I think it would be easy for them to write their own function for that.
I would have said "let's do as the std library does for tuples and arrays" but unfortunately tuples and arrays don't implement Display.

This looks great to me. Would you like to add anything before I merge this? :)

[martin-t]

Hmm, they might not impl Display but they do impl Debug...

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6e584bde76b74bb3f0d94bbc9f8beab8

...which appears to have the same "issue" as this code, same with tuple structs. So if std is ok with this behavior, i guess it's fine here.

This looks great to me. Would you like to add anything before I merge this? :)

Nope, i think it's done.

[yoanlcq]

Great! I'll merge and push a new version. Thanks.