j6k4m8 / goosepaper

generate and deliver a daily newspaper to you or your remarkable tablet
Apache License 2.0
271 stars 24 forks source link

Styles as plugin #74

Closed zerone01y closed 2 years ago

zerone01y commented 2 years ago

The styles were hard-coded in the package; reading them from a styles folder will make goosepaper much easier to customize.

These codes basically do one thing: look for a user-defined style in styles folder, and read it on-the-fly. It keeps one hard-coded style for robustness (originally FifthAvenue; now it will only be used when no/wrong style is configured).


I closed the previous PR as it was a bit messy. Glad you like this!

I have a small piece here for instructions but it is not included in this PR, as I later found out you have a doc for customising (btw we should put a link to this doc in readme). But just let me know if you want to add the instructions.

j6k4m8 commented 2 years ago

This is super awesome — can you run black on the new code so that it matches the existing Goosepaper style?

I think this is ready to go, though we may need to add these files to the manifest so that they're included when we pip-install. I'm not at a computer right now but I think that pip-installing goosepaper without adding these non-python files to a MANIFEST.in file will break.

So amazing, thank you for this great change :)

zerone01y commented 2 years ago

Thanks for the great suggestion! I'll look into them.

The code requires to mount the styles folder and a goosepaper.json file, or to run the command in a directory containing these files after pip install. This will make it easier to customize, and that's why I put the styles folder outside the goosepaper package. With these aims, I feel it is fine to exclude the non-python files. After all, a default CSS script is kept in 'styles.py' for people who don't want to customize it.

Let me know what you are thinking of the issue.

j6k4m8 commented 2 years ago

This is neat — I think it's fair game to include the styles directory in the main repository though! I'm supportive of allowing users to register their own styles, but I think it's fair for us to have a few "built-in" themes. Thoughts on that?

zerone01y commented 2 years ago

Please check again!

zerone01y commented 2 years ago

I noticed some irrelevant code was incorrectly included in the commits and broke the test. Already removed it, please test again.

codecov[bot] commented 2 years ago

Codecov Report

Merging #74 (77f559e) into master (39f6f0b) will decrease coverage by 1.43%. The diff coverage is 27.90%.

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
- Coverage   34.71%   33.28%   -1.44%     
==========================================
  Files          19       19              
  Lines         674      688      +14     
==========================================
- Hits          234      229       -5     
- Misses        440      459      +19     
Impacted Files Coverage Δ
goosepaper/styles.py 25.00% <25.71%> (-32.70%) :arrow_down:
goosepaper/goosepaper.py 33.60% <28.57%> (-0.30%) :arrow_down:
goosepaper/storyprovider/__init__.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

zerone01y commented 2 years ago

Definitely!

j6k4m8 commented 2 years ago

Thank you @zerone01y!!