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

Configure colorama communicator via front-end yaml config file #91

Open
GijsTimmers opened this issue Nov 20, 2015 · 6 comments
Open

Configure colorama communicator via front-end yaml config file #91

GijsTimmers opened this issue Nov 20, 2015 · 6 comments
Assignees
Labels

Comments

@GijsTimmers
Copy link
Owner

Seems to be a built-in:

https://landscape.io/github/GijsTimmers/kotnetcli/14/modules/kotnetcli/communicator/fabriek.py#L50

We're probably better off using something else. Some suggestions:

  • DEFAULT_COLORAMA_COLOURS (British spelling)
  • COLORAMA_DEFAULT_COLORS (Switch word order)
@jovanbulck
Copy link
Collaborator

Ok, looking back, this constant should probably be defined in coloramac.py. A ColoramaCommunicator should then take an optional colorlist as an argument. If this argument is not present, fall back to THE_DEFAULT_COMMUNICATOR_COLORS.

Moreover, the way colors are now passed in the front-end -- via cli options -- is too complicated and not scalable. A better option would probably be to parse a yaml config file, if present. When creating a coloramacommunicator, the front-end could then pass on an optional color list to the fabriek.

@GijsTimmers
Copy link
Owner Author

A ColoramaCommunicator should then take an optional colorlist as an argument. If this argument is not present, fall back to THE_DEFAULT_COMMUNICATOR_COLORS.

Sounds good.

A better option would probably be to parse a yaml config file, if present.

Agreed. I will work on this.

@GijsTimmers
Copy link
Owner Author

I created a yaml config file. It's not yet used. Please import as follows:

with open("kotnetcli.conf", "r") as f:
    configuration = yaml.load(f)

print(configuration)
{'FAIL': 'red', 'style': 'bright', 'OK': 'green', 'institution': 'kuleuven', 'WAIT': 'yellow'}

@jovanbulck
Copy link
Collaborator

I created a yaml config file

Ok, looking good. For clarity, we might want to adhere to a hierarchical syntax for communicator-specific options. e.g.

## colorama communicator-specific options
coloramac.OK:    green
coloramac.WAIT: yellow
coloramac.FAIL:  red
coloramac.style:  bright

@GijsTimmers
Copy link
Owner Author

I see what you mean. A problem with your proposal is that a user who wants to change the colours does not necessarily know what colorama or even a Communicator is. I don't think every user looks at the source code as often as we do. In that case, OK = ... is easier than coloramac.OK = .... Maybe this will suit both the novice and the advanced user:

colors:
    OK:    green
    WAIT:  yellow
    FAIL:  red
In [7]: with open("kotnetcli.conf", "r") as f:
    opties = yaml.load(f)

In [8]: opties
Out[8]: 
{'colors': {'FAIL': 'red', 'OK': 'green', 'WAIT': 'yellow'},
 'institution': 'kuleuven',
 'style': 'bright'}

@jovanbulck
Copy link
Collaborator

Ok, apart from end-user convenience, we'll have to guard cohesion here. That is, ideally the front-end should not know too much about Communicators. That is why they are created through a Fabriek.

We might be able to use your idea of a colors key-value array in the yaml file as a generic mechanism to pass user-defined options on communicator creation. The front-end could simply pass the key-value array that belongs to the chosen communicator option to the Fabriek which then passes it on to the new Communicator object. The front-end does not "understand" the contents of this array, only passing it on...

@jovanbulck jovanbulck changed the title Rename DEFAULT_COLORAMA_COLORS to something else Configure colorama communicator via front-end yaml config file Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants