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

Added .ini extension check to default config #2324

Merged
merged 7 commits into from
Dec 23, 2020

Conversation

kamui-fin
Copy link
Contributor

@kamui-fin kamui-fin commented Dec 23, 2020

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Other: Replace this with a description of the type of this PR

Description

Loads config.ini if config could not be found.

Related Issues & Documents

Closes #2323

@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #2324 (6f1aef9) into master (218911c) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2324      +/-   ##
=========================================
- Coverage    7.89%   7.89%   -0.01%     
=========================================
  Files         142     142              
  Lines       10219   10225       +6     
=========================================
  Hits          807     807              
- Misses       9412    9418       +6     
Flag Coverage Δ
unittests 7.89% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/utils/file.cpp 29.09% <0.00%> (-1.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 218911c...6f1aef9. Read the comment docs.

Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

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

Wow, that was really fast. Thanks 😃

I think this slightly changes (in a backwards incompatible way) how config files are loaded.

Before, if $XDG_CONFIG_HOME/polybar/config didn't exist (but the env variable did), it would try $HOME/.config/polybar/config, now the function directly returns an empty string.

Please also add these changes to the changelog (https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog)

These changes also need to be reflected in doc/man/polybar.5.rst, in the first section.

@kamui-fin
Copy link
Contributor Author

kamui-fin commented Dec 23, 2020

I think this slightly changes (in a backwards incompatible way) how config files are loaded.

Ah I see what you mean. It looks like I might've went wrong there when refactoring the code. I will go ahead and fix this

Please also add these changes to the changelog (https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog)
These changes also need to be reflected in doc/man/polybar.5.rst, in the first section.

Sorry, I forgot to do that 😅. I will upload the changes shortly.

Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

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

Awesome. Looks like it works now :)

Please remove the extra newlines in CHANGELOG.md again and I'll merge this.

CHANGELOG.md Outdated
@@ -1,4 +1,5 @@
# Changelog

Copy link
Member

Choose a reason for hiding this comment

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

The lack of newlines was intentional because that's also how keep a changelog does it. Please remove them again.

Copy link
Contributor Author

@kamui-fin kamui-fin Dec 23, 2020

Choose a reason for hiding this comment

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

Sorry about that, my IDE most likely auto formatted it

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I thought as much ;)

Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks a lot 🚀

@patrick96 patrick96 merged commit 89a723a into polybar:master Dec 23, 2020
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.

Add .ini extension to default config file name
2 participants