Closed pthariensflame closed 6 years ago
Has there been any update on this? I'd be really interested in using skylighting with an ANSI backend.
I got distracted, and then preoccupied, for a little while. I'd still like to work on this, but not much has been done yet. Sorry! :/
@jez @jgm This is now ready to be looked at seriously. It may need additional work to get the colors correct in all cases, but it works (and without error) on the basic tests I tried it with. It should be visually tested on a variety of terminals and environments, if possible; unfortunately, I can't do that myself, so I will have to rely on others for that task.
This works perfectly—in a terminal that supports truecolor
. We need to come up with a good way of falling back to 256color
or 16color
modes.
I tried it with skylighting -t ansi bin/extract.hs
and got something very strange:
This is in macOS terminal in xterm-256 color mode. With the espresso
style, I got green on green.
@jgm Yeah, that's a symptom of macOS’s Terminal.app not supporting truecolor
mode, only 256color
mode. I mentioned that above as something that needs to be fixed.
Thanks for the pointer to that gist! It looks helpful.
@jgm @jez This is now ready to be used in 16-color, 256-color, or true-color modes! The new -C
command line flag (long form --color-level
) will accept any of 16
, 256
, or true
to force a particular coloring mechanism, or it can accept auto
(the default) which will cause the skylighting
executable to attempt to detect the highest level of color support in the current terminal.
TL;DR: This should now work correctly on any standards-conforming ANSI terminal at all. (It probably won't work on Windows outside of WSL, but…)
Looks terrific!
Suggestions:
Unless there's a really compelling reason to put ANSI support under a flag, I'd rather just make it unconditional. I think it's considered bad practice to make the API of a library depend on flag settings. As far as I can see, ansi-terminal isn't a problematic dependency -- or is there something I don't know?
Can you rebase this a bit and consolidate the changes into one or more logically related chunks? ALternatively, I can just squash everything into one commit when merging this, but some of the changes could be sensibly put in different commits (e.g. to the Makefile).
Why don't we make ANSI output the default, instead of HTML, for the skylighting
program?
I will make all of those changes, although I'm not particularly skilled at rebasing, so I might just end up putting everything in a single commit for lack of knowledge of any other way to do it.
I had made ANSI support a conditional flag out of a perhaps misguided wish not to bloat the library for users who don't care about ANSI rendering, but in retrospect that ship has already sailed with LaTeX and HTML both being present even for users who only care about one or the other. As far as ansi-terminal
and colour
as dependencies, I don't know of any problems either.
@jgm Ok, done. For lack of expertise, I've put everything in a single commit. The default formatting for the executable is now ANSI. All conditional compilation that was previously added has disappeared.
LTS 9 doesn't have a new enough version of ansi-terminal
. This is why the build keeps failing. (LTS 10 is totally fine.)
Great! Just a couple more tweaks. Could you update the commit message with a more detailed description of the changes to the library's API? (Names of new module, new exported function(s), any other changes.) That will make it much easier to write the changelog.
And, I think adding ansi-terminal-0.8
to the extra-deps
list of stack.yaml
should fix the build failure. (This tells stack to fetch it from Hackage.) Alternatively, and perhaps better, we could have a stack.lts9.yaml
with this change and lts-9.whatever
, and use it in Travis with --stack-yaml
. That's what I do in pandoc.
@jgm Done! I used the stack.lts9.yaml
approach. Hopefully the commit message is a good enough description now.
Excellent, many thanks!
\o/ great work! Can't wait to try this out.
On Thu, Jan 11, 2018 at 8:26 PM John MacFarlane notifications@github.com wrote:
Excellent, many thanks!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jgm/skylighting/pull/22#issuecomment-357140199, or mute the thread https://github.com/notifications/unsubscribe-auth/AFSaVL442zXN7uXNBQ1HoC9kQcKUKfIVks5tJt8RgaJpZM4QSmG0 .
This is a pull request that fixes #21. It implements ANSI terminal highlighting via the
ansi-terminal
package.