Skip to content

Conversation

hamskerpir
Copy link

Hey,
my usecase was that color that I was using for my terminal prompt is same that is hardcoded in yq, so I've decided add a bit of flexibility to the tool.
Unfortunately, I did not find a config structure, and if it does not exist, I think I'm not the person that should set such change in repository :D

Anyway, here is my small changes to configure output via env vars.

@hamskerpir
Copy link
Author

image

@mikefarah
Copy link
Owner

I agree with @ccoVeille comments 👍🏼

@hamskerpir
Copy link
Author

sorry for long inactivity

Comment on lines +40 to +46
for envVar, configField := range colorMappings {
if colorStr := os.Getenv(envVar); colorStr != "" {
if attr, err := parseColorAttribute(colorStr); err == nil {
*configField = attr
}
}
}

Choose a reason for hiding this comment

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

I tend to prefer code with less branches

Suggested change
for envVar, configField := range colorMappings {
if colorStr := os.Getenv(envVar); colorStr != "" {
if attr, err := parseColorAttribute(colorStr); err == nil {
*configField = attr
}
}
}
for envVar, configField := range colorMappings {
colorStr := os.Getenv(envVar)
if colorStr == "" {
continue
}
attr, err := parseColorAttribute(colorStr)
if err != nil {
continue
}
*configField = attr
}

Also, I feel like it could be frustrating for someone if they try to use an unsupported value, they won't know it failed. I would report the error to user

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.

3 participants