Fix number implementations are never used #24
Conversation
added num macro for generating trait implementations
|
absolut no work, it was the last commit. |
src/fmtnum.rs
Outdated
| @@ -123,3 +123,13 @@ macro_rules! fmtfloat { | |||
| } | |||
| })*) | |||
| } | |||
|
|
|||
| macro_rules! fmttype { | |||
There was a problem hiding this comment.
is this the right place for this?
There was a problem hiding this comment.
it generates the definition of what formatter should be used for which type of numbers, sounds good to me, but I'm not afraid to move it elsewhere if you want me to.
|
I ended up splitting this into two parts and keeping the old behavior and adding an _ext function for formatting numbers and custom types. I've discussed this with a good friend and we see no other way for this. But different from the previous commitment, basic usage should be the same. |
|
Another solution could be to offer a macro that implements the trait for types that already have a Display impl. We still could use things like derive macros or feature flags, but I don't see anything that brings us closer to the ideal solution. |
|
after consideration, I think this should be a breaking change considering that it is fixing a bug. I'm also remembering some of the problem space. The main issue is that Display takes in a struct Anyway, all water under the bridge. On that point though, your
Thoughts? |
|
Sounds good, then let's get this on the road:
|
|
Sounds good, you rock! |
|
same legacy convention for formatter? eg format_display? |
|
You mean the |
| }; | ||
| strfmt_map(fmtstr, &formatter) | ||
| } | ||
|
|
||
| pub trait Format { |
There was a problem hiding this comment.
Sorry, I'll make this as clear as I can explain with my limited English.
The marked trait (line 96) is defining functions for str and String where usage would be "{Zero}".format(&vars).
By adding a legacy function here named strfmt_display it would make sense to add a similar extension to allow a usage like "{Zero}".format_display(&vars) as either the original signature would change from format<K, D: fmt::Display>(&self, vars: &HashMap<K, D>) -> Result<String> to format<K, D: DisplayStr>(&self, vars: &HashMap<K, D>) -> Result<String> or the body would change and default to use the legacy code.
Don't hesitate to ask, sometimes I think way too complicated and by reading these lines I wrote I'm not sure if it gets clear what problem ist the cause for this..
There was a problem hiding this comment.
Thanks for clarifying. I agree with you, there should be two functioned defined by Format:
trait Format {
fn format<K, D: DisplayStr>(&self, vars: &HashMap<K, D>) -> Result<String>
fn format_display<K, D: Display>(&self, vars: &HashMap<K, D>) -> Result<String>
}
~ renamed DisplayStr::do_format to DisplayStr::display_str ~ renamed fmttype macro to display_str_impl moved display_str_iml to lib.rs reordered lib.rs for readability + added legacy versions for strfmt (strfmt_display) and format (format_display) + deprecated legacy versions strfmt_display and format_display + added documentation + added tests for legacy versions to keep functionality for easier migration
|
Okay, so I was realizing that one big commit wasn't one of my greatest ideas, sry. Second I added some documentation and deprecation marks. As these changes contain a change of the API I thought it should increment the minor number at least. |
~ made examples simpler
|
pushed to crates.io after a few minor fixes from me |
|
ah, the version number. great, that was intended by the way, as it's your project and therefore your decision when things will get a release. The next thing on my list is the refactoring to remove warnings. |
|
I ran |
These commits contain a new trait to use the correct types when formatting.
This closes #23.
while writing this I think about mixed values and I'm unsure if there are downsides that might follow this usage.
Anyway, while writing I noticed some warnings that could be removed without any downside, while improving readability, moving on to the newer syntax.