Allow setting a custom string value for number based enums

[nieomylnieja]

I have the following code:

// ENUM(v1 = 1)
type Version int

The full name of the version is actually x/v1, which if used directly is kind of ugly, especially since the special characters cannot be part of the variable name.
While I can use string based enum to get around it:

// ENUM(v1 = x/v1)
type Version string

I'd love to be able to provide a string representation override to the int version with a flag like --stringOverride='v1:x/v1' or semantics inside the comment block itself.

[nieomylnieja]

unrelated, but I guess I'll pose the question here, @abice is this intentional? It's introducing a security breach potential :/

[abice]

@nieomylnieja no,that is not intentional... I'll review the settings today. Thanks for bringing it to my attention!

I however can't require a review for merging, because no one reviews the PRs that I make 😁

Though I suppose I can bypass as admin, that'd probably work

[nieomylnieja]

in settings for the protected branch you can check this checkbox and exclude yourself from the "Require approvals" rule :)

[nieomylnieja]

@abice and how about the actual feature request I made here? :D
Here's a real life example:

[abice]

Sorry about that!

Do aliases not fit the bill?

--alias=gte:GreaterThanEqual, gt:GreaterThan

ENUM(
gt
gte
)

I'll try to think of how that might fit into the definition block, but it doesn't have an immediate solution in my mind

[nieomylnieja]

From what I understand, wouldn't this generate a const named OperatorGt and in the ParseOperator it would also accept "GreaterThan" value? That's not exactly what I want, since I want to keep OperatorGreaterThan name and accept gt exclusively (I don't want to accept GreaterThan). Anyhow, I don't seem to be able to make it work at all:

Generating:

//go:generate ../../bin/go-enum --alias=gte:GreaterThanEqual,gt:GreaterThan

// Operator is allowed comparing method for labeling sli
// ENUM(gt, gte)
type Operator int16

It yields:

// Code generated by go-enum DO NOT EDIT.
// Version: 0.5.6
// Revision: 97611fddaa414f53713597918c5e954646cb8623
// Build Date: 2023-03-26T21:38:06Z
// Built By: goreleaser

package v1alpha

import (
	"fmt"

	"github.com/pkg/errors"
)

const (
	// OperatorGt is a Operator of type Gt.
	OperatorGt Operator = iota
	// OperatorGte is a Operator of type Gte.
	OperatorGte
)

var ErrInvalidOperator = errors.New("not a valid Operator")

const _OperatorName = "gtgte"

var _OperatorMap = map[Operator]string{
	OperatorGt:  _OperatorName[0:2],
	OperatorGte: _OperatorName[2:5],
}

// String implements the Stringer interface.
func (x Operator) String() string {
	if str, ok := _OperatorMap[x]; ok {
		return str
	}
	return fmt.Sprintf("Operator(%d)", x)
}

// IsValid provides a quick way to determine if the typed value is
// part of the allowed enumerated values
func (x Operator) IsValid() bool {
	_, ok := _OperatorMap[x]
	return ok
}

var _OperatorValue = map[string]Operator{
	_OperatorName[0:2]: OperatorGt,
	_OperatorName[2:5]: OperatorGte,
}

// ParseOperator attempts to convert a string to a Operator.
func ParseOperator(name string) (Operator, error) {
	if x, ok := _OperatorValue[name]; ok {
		return x, nil
	}
	return Operator(0), fmt.Errorf("%s is %w", name, ErrInvalidOperator)
}

[abice]

Fair enough, I'm on my phone, so couldn't double check the results.

If you have an idea of how you'd like to see it implemented that would be backwards compatible with everything else, I'm open to it.

I'll try the alias thing myself later and see what the issue would be, but I also get that it's more of a per enum thing.

I'm terms of the prefix, I think you can strip that off of the generated enum as well with an option.

[nieomylnieja]

Appreciate the swift response :)

I know about the prefix, it's not a problem for me here though :)

As to the semantics of such solution, I feel like for now, adding a new flag would solve it without any changes to the semantics to the inline definitions.
However, maybe it's time to rethink them entirely? I've seen a request somewhere to define these enums with file config and I agree with your approach, the closer it lives to the code the better. But maybe we could get the best of both worlds somehow? I'd wager YAML provides by far the easiest human friendly format out there, it's list syntax in particular would come in handy here, but having to manage it inside a comment smells tedious, although it's not uncanny --> Nix embedding file configs as raw strings. Maybe we could somehow adhere to https://go.dev/doc/comment lists and leverage doc comments formatting?
I also see yet another solution here, for more complex cases, simply allow the user to write the const block of the enum itself and allow specifying comments there, while also taking flags into consideration, example (this also adheres to stringer behaviour):

const (
  OperatorGreaterThan Operator = iota + 1 // gte
  OperatorGreaterThanEqual                          // gt
  ...
)

And truth be told the latter is the most convincing to me, it feels like further extending ENUM comment block semantics, while certainly possible, might not be the best idea in terms of added complexity in ambiguity. I think after all, the enum declaration should be simple enough for anyone to make sense of it, without go-enum familiarity.

[abice]

While allowing the user to define the iota themselves may eliminate some other issues that arise (bitmasks for one), I'm not sure that would be any easier to implement, and would likely make the codebase even more difficult, as now there's 2 places to look for information, rather than just a single comment block.

Personally, the idea that I came up with would mirror (or mimick) struct tags.

ENUM(
gte = ,alias:"GreaterThanEqual"
gt = ,alias:"GreaterThan"
)

I'm still gonna give it some more time to settle before choosing a direction though.

[abice]

@nieomylnieja As a follow up to this... I do think there is some benefit in allowing someone to specify their own const/iota definition, and have the start of something to do this, but it's a lot more complicated to work out than what I have currently. Will definitely need more time to get something working around this idea. Sorry for the delay, but thanks for the patience!