search

Sixdsn / terra-terminal

Terra is a GTK+3.0 based terminal emulator with useful user interface, it also supports multiple terminals with splitting screen horizontally or vertically. New features have been added since September 2013. It's a pretty new and experimental project, if you want to contribute just checkout and try.
GNU General Public License v3.0
7 stars 3 forks source link

Rename the layout config "disabled" to "enabled" #79

Closed SchnWalter closed 9 years ago

SchnWalter commented 9 years ago

The method ConfigManager.get_conf() will return None for undefined configuration options.

Currently we have:

def get_screen(name):
    if (ConfigManager.get_conf(name, 'enabled') == False):
        return None

And for example if you make a small changes to:

def get_screen(name):
    if not ConfigManager.get_conf(name, 'enabled'):
        return None

The program will stop working because None != False but both are evaluated as False in logical expressions.

Instead of fixing only this one small statement I suggest renaming the property to "disabled" and have it optional, meaning if it is missing, the layout is enabled by default.

I have a commit prepared, but I'm waiting for the previous Pull Request to be merged into the develop branch.

Sixdsn commented 9 years ago

Hi,

I agree with you. I didn't had time to finalize my review of your pull request but i'm gonna work on it this weekend. About that, think to modify the way it's saved in the configuration file.

Thanks,

SchnWalter commented 9 years ago

About, configuration files: I've played around with a new dictionary based ConfigManager, seems to work nice, but I've started to also change the code that is using the ConfigManager, and the config structure and then everything went down the hill when I started changing the logic and stuff. So I'll keep it as an example for later usage.

First I'm going to start committing clean up to various parts of the code. Once those are in, I'm going to push copy the new ConfigManager, and start updating the usage. Here how it will look like:

# Gets the user configured global_key, if there is none, it provides the default value.
global_key = TerraHandler.config['shortcuts']['global_key']

And for layouts I have added @property methods, and with those the code is more readable.