koiuo / randrctl

Profile based screen manager for X
GNU General Public License v3.0
47 stars 12 forks source link

KeyError: 'other' when running randrctl dump #13

Closed eddie-dunn closed 6 years ago

eddie-dunn commented 6 years ago
% randrctl dump -e home
WARNING No home directories found among ['/home/user/.config/randrctl', '/etc/randrctl/']
WARNING Creating home under /home/user/.config/randrctl
Traceback (most recent call last):
  File "/home/user/.local/bin/randrctl", line 11, in <module>
    main()
  File "/home/user/.local/bin/randrctl", line 7, in main
    Main().run()
  File "/home/user/.local/lib/python3.6/site-packages/randrctl/__main__.py", line 115, in run
    self.randrctl = factory.get_randrctl()
  File "/home/user/.local/lib/python3.6/site-packages/randrctl/ctl.py", line 274, in get_randrctl
    config = self._load_config_files()
  File "/home/user/.local/lib/python3.6/site-packages/randrctl/ctl.py", line 229, in _load_config_files
    yaml_files = [config_file for config_file in config_files[use] if config_file.endswith('yaml')]
KeyError: 'other'

In case it matters, I installed randrctl via pip like so:

pip3 install --user randrctl

Crash encountered in Ubuntu 18.04.

koiuo commented 6 years ago

Thanks for the report. As an immediate workaround, please do

mkdir -p ~/.config/randrctl
touch ~/.config/randrctl/randrctl.yaml

I'm not 100% but this likely fix the issue.

I'll look into the root cause this weekend.

eddie-dunn commented 6 years ago

Unfortunately, creating an empty yaml file didn't work (randrctrl already creates the folder ~/.config/randrctl if it doesn't exist already).

I skimmed through the code, and it seems to assume that a key exists when it in reality doesn't here: https://github.com/edio/randrctl/blob/master/randrctl/ctl.py#L229

The _configs method in the same class never seems to create the keys you are looking for, perhaps because self.homes isn't set correctly when I run it?

Either way, when you create the dict maybe you should define it as configs = { self.OTHER: [] } or something? Or just use config_files.get(use, ()) on lines 229 and 231?

I don't have time to test my suggestions right now, so I might be off, though.

koiuo commented 6 years ago

@eddie-dunn I haven't wrote this code and don't recall the details.

Eventually, I was going to refactor it and eliminate ini files support in favor of yaml (since we have yaml dependency anyway).

Seems that the time has come.

As I said, I'll look into the issue this weekend. In case if you have a fix or an idea how to improve things, feel free to create a PR. But I do understand that the code right now is not in the best shape, so probably a lot has to be rewritten entirely.

koiuo commented 6 years ago

@eddie-dunn I made some re-factoring and it issue must have fixed the issue. Please check if it works and please let me know if it doesn't.

Thanks again for reporting!

eddie-dunn commented 6 years ago

@edio I can confirm that your refactoring patch fixed the problem. Thanks!