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

xdg support for .mackup.cfg and .mackup #632

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

xdg support for .mackup.cfg and .mackup #632

wants to merge 5 commits into from

Conversation

mwilliammyers
Copy link
Contributor

Implements #473

@@ -94,7 +94,20 @@ def get_config_files():
# Configure the config parser
apps_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)),
APPS_DIR)
custom_apps_dir = os.path.join(os.environ['HOME'], CUSTOM_APPS_DIR)
xdg_config_home = os.environ.get('XDG_CONFIG_HOME')
if xdg_config_home:
Copy link
Owner

Choose a reason for hiding this comment

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

it's a lot of copy pasta. Can you add a util that you can use to look for a file in XDG_CONFIG_HOME if set, else in HOME? And use it everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very true. Will do.

@mwilliammyers
Copy link
Contributor Author

Would you be ok with 2 utils?

  1. Validate $XDG_CONFIG_HOME (that it exists as a path and is within ~)
    • This would be used in my previous PR when parsing the .cfg file
      i.e. in replace of: L57-L67
  2. Return $XDG_CONFIG_HOME (after using the util 1 above to validate the path) if set; else $HOME
    • This would be used instead of the two horrendous copy and pasted chunks on this commit

@mwilliammyers mwilliammyers reopened this Aug 30, 2015
@mwilliammyers
Copy link
Contributor Author

@lra I updated this with the util functions as requested. I hope this isn't too much to merge at once...


# Directory that can contains user defined app configs
CUSTOM_APPS_DIR = '.mackup'
if os.environ.get('XDG_CONFIG_HOME'):
Copy link
Owner

Choose a reason for hiding this comment

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

sorry constants are constants, no code in here.
You can have different constants and pick one or another in functions defined elsewhere (e.g. utils)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I kinda felt that way too so I put it in a separate commit. Ill fix that.

@lra
Copy link
Owner

lra commented Sep 2, 2015

I'll give it another review pass once those comments are dealt with, thanks!

William Myers added 2 commits September 6, 2015 22:12
* upstream/master:
  changelog
  Update CHANGELOG.md
  Update ubersicht.cfg
  Update CHANGELOG.md
  Update bettertouchtool.cfg
  Add support for AppCode 3.2
  Update README.md
  Add configuration for MacDive
@mwilliammyers
Copy link
Contributor Author

@lra I made the suggested changes (at least I think I got all of them). Let me know if you want me to rebase/squash...

@mwilliammyers
Copy link
Contributor Author

@lra Just checking up on this... is there something I missed?

@lra
Copy link
Owner

lra commented Sep 28, 2015

no idea, I need to spend time reviewing it.

@laughedelic
Copy link
Contributor

Any updates here?

@TomatoMus
Copy link

@lra Are there any reviews or merge plans for this PR?

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

Successfully merging this pull request may close these issues.

None yet

4 participants