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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ability to use custom icon styles + dark icon set. #638

Merged
merged 4 commits into from Jan 18, 2017
Merged

Ability to use custom icon styles + dark icon set. #638

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6123eab
Add dark icon style
cofi89 Jan 3, 2017
eb09b68
doc: document cola.iconstyle
cofi89 Jan 3, 2017
a9bb676
Add ability to load custom icons
cofi89 Jan 3, 2017
d65eb0b
setup: install dark icon set
cofi89 Jan 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions cola/gitcfg.py
View file
Open in desktop
Expand Up @@ -413,3 +413,7 @@ def color(self, key, default):
except Exception:
r, g, b = struct.unpack(struct_layout, unhexlify(default))
return (r, g, b)

def icon_style(self):
theme = self.get('cola.iconstyle', None)
return theme
8 changes: 7 additions & 1 deletion cola/icons.py
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@

from qtpy import QtGui

from . import gitcfg
from . import qtcompat
from . import resources
from .compat import ustr
Expand All @@ -31,9 +32,14 @@
'.cxx': 'file-code.svg',
}

cfg = gitcfg.current()
style = cfg.icon_style()

def install():
icon_dir = resources.icon_dir()
if not style or style == "light":
icon_dir = resources.icon_dir()
else:
icon_dir = resources.icon_dir(style)
Copy link
Member

Choose a reason for hiding this comment

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

tiny tweak: for keyword arguments, prefer doing icon_dir(style=style) at the callsite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

馃憤 Will do, thanks for the correction.

Copy link
Member

@davvid davvid Jan 18, 2017 •
edited

Choose a reason for hiding this comment

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

I think this is really the main change that I keep coming back to as a slightly unfortunate change in the relative dependencies. Previously the icons module was more pure, and didn't import anything, and we didn't have to call into git so early (during module import) so that we can defer a few things.

Sorry it's taken so long to get back to this one, but it's become a little clearer with time. Would it be okay if we made this something that's controlled via the environment instead?

There's already precedent for a few of these in the ENVIRONMENT VARIABLES section of docs/git-cola.rst, namely GIT_COLA_SCALE which is directly related to the UI. The reason these are done via the environment is that they affect the critical startup code paths that we've tried to keep as minimal as possible.

Would it be okay if we changed the style to be a lookup of os.environ.get('GIT_COLA_ICON_THEME', 'light')? Presumably we can then make the style optional argument into a required argument, and special-case it inside of icon_dir() instead.

I may merge this soon with these changes applied locally, which hopefully won't be too much of a hassle to upgrade to.

Example .bashrc/.xsessionrc:

GIT_COLA_ICON_THEME=dark
export GIT_COLA_ICON_THEME

Copy link
Member

Choose a reason for hiding this comment

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

Just following up -- not sure if you noticed but I ended up keeping the config-driven approach, and ended up augmenting it with --icon-theme=<theme> as well as allowing GIT_COLA_ICON_THEME in the environment. The only other minor difference is that the config variable is named cola.icontheme whereas it was originally cola.iconstyle.

qtcompat.add_search_path('icons', icon_dir)


Expand Down
11 changes: 9 additions & 2 deletions cola/resources.py
View file
Open in desktop
Expand Up @@ -57,9 +57,16 @@ def share(*args):
return prefix('share', 'git-cola', *args)


def icon_dir():
def icon_dir(style=None):
"""Return the path to the style dir within the cola install tree."""
return share('icons')
if style:
style_dir = share('icons', style)
if os.path.isdir(style_dir):
return style_dir
else:
return share('icons')
else:
return share('icons')


def config_home(*args):
Expand Down