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

fix(proto): implement fmt.Formatter for Field and Value #521

Merged
merged 1 commit into from
Feb 9, 2025

Conversation

muktihari
Copy link
Owner

@muktihari muktihari commented Feb 9, 2025

There are two objectives that this PR solves by implementing fmt.Formatter for proto.Field and proto.Value.

  1. Reduce gotcha when printing proto.Value, as proto.Value has the same implementation of fmt.Stringer, but we treat it as a special case method to retrive string value, rather than the underlying value formatted as a string. Example:

    val := proto.Uint8(10)
    fmt.Println(val)       // We might expect 10, but it prints "" since it's not a string
    fmt.Println(val.Any()) // Previous workaround, we must call Any() method

    And thing gets trickier when we print a type containing proto.Value such as proto.Field or maybe if we print the whole proto.FIT itself, as it will trigger fmt to call (proto.Value) String() string method. Related issue: Totals not complete, most of the unknown fields are missing #520

    Now we can just do:

    val := proto.Uint8(10)
    fmt.Println(val)
    // Output:
    // 10
  2. Enable more verbose printing format for proto.Field for debugging purposes. Since proto.FieldBase is embedded in proto.Field as a pointer, fmt prints the pointer address to proto.FieldBase instead of its value. In general, we need to know the description of the field such as field number, field name, type etc when printing proto.Field. Now, we have it:

    field := factory.CreateField(mesgnum.Record, fieldnum.RecordDistance).WithValue(uint32(7))
    
    fmt.Println(field)
    // Output:
    // old: {0x59ee20 7 false}
    // new: {(0x59ee20)(&{distance 5 uint32 uint32 false true 100 0 m [] []}) 7 false}
    
    fmt.Printf("%+v\n", field)
    // Output:
    // old: {FieldBase:0x59ee20 Value:7 IsExpandedField:false}
    // new: {FieldBase:(0x59ee20)(&{Name:distance Num:5 Type:uint32 BaseType:uint32 Array:false Accumulate:true Scale:100 Offset:0 Units:m Components:[] SubFields:[]}) Value:7 IsExpandedField:false}

    We keep the pointer address in the formatted string in case it matters.

@muktihari muktihari added the bug Something isn't working label Feb 9, 2025
@muktihari muktihari self-assigned this Feb 9, 2025
@muktihari muktihari added this to the v0.24.5 milestone Feb 9, 2025
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5515a76) to head (8746aa9).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #521   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         4618      4641   +23     
=========================================
+ Hits          4618      4641   +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@muktihari muktihari changed the title fix(proto): implement fmt.Formatter for proto.Field and proto.Value fix(proto): implement fmt.Formatter for Field and Value Feb 9, 2025
@muktihari muktihari enabled auto-merge (squash) February 9, 2025 11:31
@muktihari muktihari disabled auto-merge February 9, 2025 11:32
@muktihari muktihari merged commit eea67ce into master Feb 9, 2025
5 checks passed
@muktihari muktihari deleted the fix/field-value-fmt-formatter branch February 9, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants