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

printf: handle Array and Map #26

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

flavio
Copy link
Contributor

@flavio flavio commented Jun 21, 2021

Handle printing of %v with Array and Map objects.

Obviously this won't cover 100% of the cases, but it's better than nothing

@flavio
Copy link
Contributor Author

flavio commented Jul 14, 2021

@fiji-flo hi, any chance to get this PR reviewed? 🥺

Copy link
Owner

@fiji-flo fiji-flo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR @flavio

Could you make it behave like golang fmt.Printf (see https://play.golang.org/p/3AR2Vlf7FWc) and remove the regex dependency?

I'll get back to you faster next time. I had some busy weeks.

src/printf.rs Outdated Show resolved Hide resolved
@flavio
Copy link
Contributor Author

flavio commented Aug 4, 2021

@fiji-flo, sorry for the late response. I was on vacation :)

I've updated the PR to address your notes. Note well, the rendering of arrays was already fine (unless I missed something), I fixed the rendering of maps.

Once you're happy with the code I can squash the 3 commits into a single one, unless you prefer to merge the branch as it is

Copy link
Owner

@fiji-flo fiji-flo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks a lot. If you could adopt the nit pick there I'll merge and create a 0.7.1 release 🥂

src/printf.rs Outdated Show resolved Hide resolved
Handle printing of "%v" with Array and Map objects.

Obviously this won't cover 100% of the cases, but it's better than
nothing :)

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
@flavio flavio force-pushed the printf-handle-array-and-map branch from 76a356b to 04ec37a Compare August 6, 2021 07:14
@flavio
Copy link
Contributor Author

flavio commented Aug 6, 2021

@fiji-flo I've accepted the change, fixed the indentation, ran the tests and then squashed everything into a single commit

Thanks for your reviews!

Copy link
Owner

@fiji-flo fiji-flo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks. I'll fix the clippy warnings (that's on me) and get it released.

@fiji-flo fiji-flo merged commit 447f416 into fiji-flo:master Aug 6, 2021
@flavio flavio deleted the printf-handle-array-and-map branch August 6, 2021 08:44
@flavio flavio restored the printf-handle-array-and-map branch August 6, 2021 08:44
@flavio
Copy link
Contributor Author

flavio commented Aug 6, 2021

@fiji-flo just out of curiosity, how do you handle release? I wanted to subscribe to release events to get notified about the 0.7.1 release, but I noticed no release has ever done and no git tags are associated to the repo 🤔

@fiji-flo
Copy link
Owner

fiji-flo commented Aug 6, 2021

I've never automated it. That's on my TODO for all my crates to move this into a github action. But I published it already.

I'll move to branches for the 0.X and tags for releases then.

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.

None yet

2 participants