search

ohler55 / ojg

Optimized JSON for Go
MIT License
857 stars 49 forks source link

Support Indexed and Keyed interfaces when removing nodes #181

Closed twelvelabs closed 2 months ago

twelvelabs commented 2 months ago

This PR adds support for the Indexed and Keyed interfaces when removing nodes from an expression.

Note: I needed to add a method for removing items from Indexed. If you'd rather avoid a breaking change, we could move RemoveValueAtIndex to a separate interface. I originally went down that route, but got stuck trying to come up with a name. MutableIndexed was the only thing I could come up with and... it doesn't really sound great to me 😬. Let me know what you prefer (or feel free to take over this PR).

Also, a belated thank you for fixing #156 ❤️! I tried out the latest release and it works great.

ohler55 commented 2 months ago

While I agree the Indexed interface should probably have had a RemoveValueAtIndex to start with I'd rather nat introduce a breaking change so separate interface for that method is probably best. Naming the interface does get trickier. RemovableIndexed doesn't really roll off the tongue but might be a reasonable choice.

Other than that the PR looks good although I did not verify coverage yet.

twelvelabs commented 2 months ago

@ohler55 Updated per your suggestion.

ohler55 commented 2 months ago

Been on vacation. Sorry for the delay. Looks good.