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

feat(p/ufmt): Support more formatting verbs #2351

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

notJoon
Copy link
Member

@notJoon notJoon commented Jun 13, 2024

Description

Support for several new formatting verbs to the ufmt package:

%x: Outputs byte values as hexadecimal strings. Supports uint8, []uint8, and [32]uint8 types.
%c: Outputs characters. Supports rune and int types.
%q: Outputs quoted strings with proper escaping. Supports string type.
%T: Outputs the type of the value. Supports various built-in types using type switching with interface{}.

Particularly in the case of byte slices, previously need to use a loop to convert each element to a string one by one, but now there is no need for that anymore.

@notJoon notJoon requested review from a team as code owners June 13, 2024 10:14
@notJoon notJoon requested review from thehowl and ltzmaxwell and removed request for a team June 13, 2024 10:14
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jun 13, 2024
Copy link
Contributor

@mvertes mvertes left a comment

Choose a reason for hiding this comment

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

Looks good to me, except a minor simplification.

examples/gno.land/p/demo/ufmt/ufmt.gno Outdated Show resolved Hide resolved
Co-authored-by: Marc Vertes <mvertes@free.fr>
Comment on lines +139 to +148
switch v := arg.(type) {
case uint8:
buf += strconv.FormatUint(uint64(v), 16)
case []uint8, [32]uint8:
for _, b := range v {
buf += strconv.FormatUint(uint64(b), 16)
}
default:
buf += "(unhandled)"
}
Copy link
Member

Choose a reason for hiding this comment

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

this code... isn't supposed to work... https://go.dev/play/p/wzaZ2BdezvX

Copy link
Member

Choose a reason for hiding this comment

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

yeah ok, it doesn't in gno, either. that's why the build is failing.

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

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

good, please merge master and fix ci.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: In Progress
Status: In Review
Development

Successfully merging this pull request may close these issues.

4 participants