search

rktjmp / lush.nvim

Create Neovim themes with real-time feedback, export anywhere.
MIT License
1.45k stars 47 forks source link

WIP savq doc codereview #7

Closed rktjmp closed 3 years ago

savq commented 3 years ago

Ok, I fixed some of the suggestions. I agree that explaining the directory structure first is better.

I also put back the additional info that doesn't really fit in the docs.

One of the things I noted that could still be improved is the usage of the words spec, theme and color scheme; or rather, we could remove the instances of theme, and replace it with spec, when talking about Lua, or color scheme, when talking about VimL (or the parsed result); or simply call it colors, like lush_colors/ instead of lush_theme.

rktjmp commented 3 years ago

I've done a bit more work, https://github.com/rktjmp/lush.nvim/tree/readme-update (cross branch diff: https://github.com/savq/lush.nvim/compare/doc...rktjmp:readme-update).

I think I might compress 2 and 3 into one section, right now it reads like you have to define your colours then the spec, when really they can be intermixed, or more interestingly apply HSL operations on group attributes.

Really I think I might drop quite a lot of it and just have the demo spec for githubbers to see what they can expect to write and then point any actual guide to docs/lush.txt and :LushRunTutorial.

savq commented 3 years ago

I think I might compress 2 and 3 into one section … apply HSL operations on group attributes

Yeah, re-reading that does make it seem like the two things are more separate than they should be.

Really I think I might drop quite a lot of it ... then point any actual guide to docs/lush.txt and :LushRunTutorial.

Sure. I actually found it quite difficult to explain why Lush is easier to use than other tools, even though it's obvious after running the tutorial. Maybe put a bigger demo video? something that shows code in a split buffer.

The rest looks pretty good to me already. I liked the idea of a script to copy the template automatically. I thought it could be added to the repo, with something to check for LUSH_NAME:

#!/bin/sh
[[ -z "${LUSH_NAME}" ]] && echo 'LUSH_NAME is not defined' && exit 1
# ... rest of the script

Other than that, I don't know if there's anything missing.

savq commented 3 years ago

I also saw that you put paq in there 👀 😄

rktjmp commented 3 years ago

Manually merged offline.