You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-themes/pull/30
-- Commit Summary --
* Create README.md
-- File Changes --
A screenshots/README.md (171)
-- Patch Links --
https://github.com/geany/geany-themes/pull/30.patch https://github.com/geany/geany-themes/pull/30.diff
codebrainz requested changes on this pull request.
Other than the comments, this looks good. Nice work.
+### [pygments.png](pygments.png)
+ + + + +### [railcasts2.png](railcasts2.png) + + + + +### [retro.png](retro.png) + + + + +### [screenshot-missing.png](screenshot-missing.png)
This one shouldn't be here.
@@ -0,0 +1,171 @@
+ +### [bespin.png](bespin.png)
What if the header linked directly to the theme file so it could be downloaded directly into the user's theme directory, and then leave the images to link to the PNG file?
One more thought, why not generate this using a script so we don't have to remember to update it whenever a theme is added? If not using a script, then the [ADDING-A-THEME](https://github.com/geany/geany-themes/blob/master/ADDING-A-THEME.md) file needs to be updated to mention adding the Markdown into this new README file.
suntong commented on this pull request.
@@ -0,0 +1,171 @@
+ +### [bespin.png](bespin.png)
OK. Let me try...
As to _"generate this using a script"_, that's something I didn't know and needs admin rights to the geany-themes repo too...
suntong commented on this pull request.
@@ -0,0 +1,171 @@
+ +### [bespin.png](bespin.png)
@codebrainz , would the following be sufficient for what you asked for (the linking directly to the theme file for downloading part)?
``` ### [bespin theme](../colorschemes.bespin.conf) ```
suntong commented on this pull request.
+### [pygments.png](pygments.png)
+ + + + +### [railcasts2.png](railcasts2.png) + + + + +### [retro.png](retro.png) + + + + +### [screenshot-missing.png](screenshot-missing.png)
You meant screenshot-missing, not `retro.png`, right?
codebrainz commented on this pull request.
+### [pygments.png](pygments.png)
+ + + + +### [railcasts2.png](railcasts2.png) + + + + +### [retro.png](retro.png) + + + + +### [screenshot-missing.png](screenshot-missing.png)
Yes
codebrainz commented on this pull request.
@@ -0,0 +1,171 @@
+ +### [bespin.png](bespin.png)
@codebrainz , would the following be sufficient for what you asked for (the linking directly to the theme file for downloading part)?
Yeah, that's what I meant.
I just meant a simple (ideally Python, since it's what's already used) script that scans the themes and prints the Markdown. The script would be run from the make file to regenerate the Markdown file whenever adding a new theme, like is already done with `mkindex.py`.
@suntong pushed 1 commit.
1cd655e85232df9cd4ccaa9f22f5c46418008c24 Update README.md
Verified that the changes are working at my end.
codebrainz approved this pull request.
This looks good to me.
As mentioned in the comments, it would be nice to have another pull request that either generates this using a Python script (rather than using additional tools/templates), or to update `ADDING-A-THEME` reminding to update this file manually.
Merged #30 into master.
@suntong I added a follow up PR #31, what do you think?
I don't know Python, but I think such screenshots generation, using Python or not, will only be a temporary solution, as a proper implementation at the server-side is the best option in the long run, since geany already has its web site, and that is where these things belong to.
All Geany is provided by volunteer contributors, so "somebody" needs to contribute the website server side (and its Django so its still Python, see [here](https://github.com/geany/www.geany.org)).
For the same reason anything that makes it easier to add themes here is useful. The more manual it is the more someone like @codebrainz has to prompt contributors to fix their pull requests every time they contribute a theme.
Maybe it came out wrong, but before I even submitted the PR, I stressed that I have no objection of server-side implementation that you proposed @elextr, and mine will be only be a temporary solution before yours' done.
Anyway, I don't know Python, so this is as far as I can help.
@suntong agree, and I put somebody in quotes to indicate not you (or me :).
The comment was so "somebody" who saw this and thought it was a good idea would know where to look for the website, its new, and being in that repo where its easy to contribute pull requests is new.
Or if they didn't want to try the server side it was clear that a script here was still acceptable.
Guys, in https://github.com/geany/www.geany.org/issues/5 I already said I will work on this and now done in: https://github.com/geany/www.geany.org/pull/8.
Just to be clear, I see what this PR added (a [screenshots/README.md](screenshots/README.md) to make browsing on Github nicer) as completely separate from the website as in geany/www.geany.org#5 and geany/www.geany.org#8. I added #31 to make this no more work than regenerating the indices and such (ie. running `make`).
@eht16 didn't see https://github.com/geany/www.geany.org/issues/5 because I'm not subscribed to that repo, but unless geany/www.geany.org#8 that also provides the ability to save the theme to a file directly from the page, users will still need to come here to get the themes, so as @codebrainz said its good to have both.
I completely agree. The preview page on the website and the preview in the README here do not compete at all. It's perfectly fine to have both.
github-comments@lists.geany.org