Skip to content
This repository was archived by the owner on Apr 19, 2024. It is now read-only.

Add support for changing the color of the different runes/prompts #141

Closed
kleoken opened this issue May 10, 2018 · 11 comments · Fixed by #222
Closed

Add support for changing the color of the different runes/prompts #141

kleoken opened this issue May 10, 2018 · 11 comments · Fixed by #222
Labels
Milestone

Comments

@kleoken
Copy link

kleoken commented May 10, 2018

Creating a CLI with this currently and would be nice if I could change the color of the QuestionIcon application wide

@AlecAivazis
Copy link
Owner

Hey @celo44 - thanks for submitting this. I don't know if I will have the time in the near future to add this but if you wanted to give it a stab, I would be happy to review a PR 🙂

@MarkusFreitag
Copy link
Contributor

MarkusFreitag commented Aug 13, 2018

I would start by making the icons configurable. Therefore I would like to evolve to icon, which is currently a string, into a struct which for now has two fields.

type Icon struct {
  Symbol string
  Color  string
}

Maybe @AlecAivazis can give some feedback about this approach. If it is accepted I will provide a PR.

@AlecAivazis
Copy link
Owner

Hey @MarkusFreitag - thanks for the input. I like the idea, although I'm not sure if providing this would be too much customizability for the library. Can you maybe share your reason for wanting to change the colors?

In general, i'm a bit wary of changing the public API for a relatively minor feature. I think I would prefer if there was a way to call something like surveyCore.SetIcon("ErrorIcon", Icon{Symbol: "!"}) which keeps the top level API the same. In this case, we would add variables for each of the colors, and SetIconwould change the values ofErrorIconto theSymbol` field and the color templates to match that field if provided.

Thoughts? Would that satisfy both of your needs?

@MarkusFreitag
Copy link
Contributor

MarkusFreitag commented Aug 13, 2018

Thanks for your feedback. It does not really changes the API. Basically I have just moved the hard-coded color definitions from the template strings into the icon struct and then call Icon.Color in the template. Also the var TemplateFuncs = map[string]interface{} works as before. I think that it also makes it easier to maintain the library, as you have such things in a central place and not spread over every template. Nevertheless I can push my changes into a branch, which might makes it easier to find a consent on this.

Can you maybe share your reason for wanting to change the colors?

Passing this question to @celo44

@MarkusFreitag
Copy link
Contributor

@AlecAivazis I put the changes into a separate branch, maybe in the next days you have some time to have a look on them. As written before the API behaviour does not change.
MarkusFreitag@ceb60e1

@bmuschko
Copy link
Contributor

@AlecAivazis Any chance we can move forward with @MarkusFreitag's commit?

@AlecAivazis
Copy link
Owner

@MarkusFreitag since we're preparing to merge the v2 release that will break the API, it's a good opportunity to use the API that you suggested - wanna give it a shot?

@AlecAivazis
Copy link
Owner

AlecAivazis commented Jun 2, 2019

#216 adds the survey.WithIcons option which looks a little something like:

survey.AskOne(prompt, &value, survey.WithIcons(func (icons *survey.IconSet) {
    icons.Question = "?"
})

This can easily accommodate a richer API like the one in your branch @MarkusFreitag

@AlecAivazis
Copy link
Owner

Alright, since this issue is the only one left that's slated for this v2 release, i'm going to go ahead and implement it so we can release this ASAP.

@AlecAivazis AlecAivazis mentioned this issue Jun 4, 2019
@AlecAivazis
Copy link
Owner

I just opened #222 which adds the ability to modify the Format of an Icon with the WithIcons option

@AlecAivazis
Copy link
Owner

This will be released in the upcoming v2 that should be available by the end of the day

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants