godlygeek / csapprox

Make gvim-only colorschemes work transparently in terminal vim
http://www.vim.org/scripts/script.php?script_id=2390
211 stars 18 forks source link

More accurate and minimal snapshots. #7

Open bukzor opened 12 years ago

bukzor commented 12 years ago

I'd really benefit from a critique of this changeset.

In particular, the colors 'fg' and 'bg' were being used too often, which was preventing some reverse and background-only color groups from functioning as intended.

I went on to do a fairly major refactor which allows me to output a more minimal and readable snapshot color scheme. I've started capturing syntax group links because some color schemes use them in their implementation (notably ir_black and zenburn).

I've noticed that my minimized konsole/eterm settings are always blank. Either I've broken it, it was already broken, or it is working but the konsole/eterm colors are never different from the xterm colors.

Future work: (please comment if you have suggestions or concerns)

Thanks in advance for your time. --buck

godlygeek commented 12 years ago

For the two refactors of the snapshot code, I'll have to look through them a bit more carefully and see what I think. In general, I'd rather not check in changes just for the sake of making them smaller and more readable - in general, I think that generated code ought to be easy to generate, rather than easy to read. It looks like there are some actual fixes to the snapshotting mixed in here as well, but I don't want to apply those without a testcase showing the change that they cause, and a careful review to make sure that they don't make any of my other testcases worse.

bukzor commented 12 years ago

Some remarks that may help:

The chief thing I've done is change the layout of the s:Highlights return value. If you can agree to that, the rest flows naturally from there. I think the layout results in more natural, readable code downstream, but it also enables two features that I'm trying to introduce.

First, I'd like to capture color-group links, as these are used in some color schemes. Ignoring them gives incorrect results.

Secondly, I'm planning a test suite, of sorts. I'd like to add a CSApproxSnapshotAll command that will capture all known color schemes under the CSApprox/colors directory (by default). If those results are deterministic (i.e. sorted) then git diff will suss out any functional changes that have been made. Another implication is that these snapshot color schemes will be added to the project, so I feel that making them small / readable is a good side effect. The reorganization of s:Highlights makes these optimizations fairly trivial.