search

kislyuk / yq

Command-line YAML, XML, TOML processor - jq wrapper for YAML/XML/TOML documents
https://kislyuk.github.io/yq/
Apache License 2.0
2.62k stars 84 forks source link

implement non-JSON highlight with pygments #67

Closed romain-dartigues closed 5 years ago

romain-dartigues commented 5 years ago

Output colorization for non-JSON output is probably often requested (as per #17).

I propose to implement a wrapper around pygments. At the same time, I fixed the case when colorization is asked on a non-JSON format (until now yq -[t|y|x] -C . file.yml would fail with Error running jq: JSONDecodeError: Expecting value: line 1 column 1 (char 0). L159-160).

Note: I'm not especially confortable with the way I implemented it as I wanted to move the minimal amount of existing yq code; I could probably do something nicer with a bigger rework.

varac commented 5 years ago

@romain-dartigues Can you please take care of the failing checks ?

romain-dartigues commented 5 years ago

Okay, I'll improve the code-coverage as soon I'll have a minute. Do you have any suggestion about the code?

kislyuk commented 5 years ago

@romain-dartigues sorry, at this point I'm not comfortable with adding pygments as a dependency or with the code introduced in colorize.py, given the fact that pygments doesn't have a meaningfully useful YAML syntax highlighter.

I agree it would be nice to have syntax highlighting like jq does, but pygments is a heavy dependency that doesn't buy us much in that regard.

kislyuk commented 5 years ago

Filed #71 to track the yq -C issue that you highlighted (thanks for that).

romain-dartigues commented 5 years ago

@kislyuk < It was an optional dependency, but I respect your decision and would have been upset to put more effort into it if you still didn't want it at the end (^_^).

I also thought about making a lightweight highlighter for yq instead of adding the somewhat heavy one that is pygments. But in favour of the dependency is it provide highlighting for others syntax than YAML (since now yq also output TOML, XML and what more in the future).

72 filled to fix -C.

kislyuk commented 5 years ago

Thanks for your attention to this @romain-dartigues. After resolving the issue with -C, I'll take another look at whether pygments may be appropriate. But the biggest issue, as I mentioned above, is that the pygments YAML highlighter just doesn't highlight things well - not nearly as well as (for example) the JSON one in jq. If we could get really nice highlighting with this dependency (and the possible bugs that may come with it), it would be worth it. But right now I'm still looking for a good YAML highlighter before we can get to the point of seriously considering integrating it.