Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax: add cmd-help #2148

Merged
merged 3 commits into from
Apr 26, 2022
Merged

Syntax: add cmd-help #2148

merged 3 commits into from
Apr 26, 2022

Conversation

victor-gp
Copy link
Contributor

As discussed in #2130.

I tried to add a syntax test but it's not highlighted because this is a syntax without auto-detection and the syntax tests scripts don't run bat --language=X.

@keith-hall
Copy link
Collaborator

I tried to add a syntax test but it's not highlighted because this is a syntax without auto-detection and the syntax tests scripts don't run bat --language=X.

Interesting that this hasn't come up as a problem before. It may be worth considering mapping to some unique filename/extension which won't clash with anything else.

@sharkdp
Copy link
Owner

sharkdp commented Apr 6, 2022

Thank you for this contribution!

For the syntax tests, you should be able to use this: https://github.com/victor-gp/cmd-help-sublime-syntax/blob/2cf2ecc0e183dcc5989cd184ef9d9cbed3e4dfb5/syntaxes/cmd-help.sublime-syntax#L4-L5, right?

Otherwise, you can also place a bat_options file in the source folder (see e.g. the Plaintext syntax). This could probably just contain --language=cmd-help.

By the way, for usage with the -l/--language, it would be nice to have a short name for this syntax I suppose? Like help instead of cmd-help maybe? This would allow using this like

git --help | bat -lhelp

Or do you have other ideas how this would be used? With a shell function a la

bat_help() {
  cmd="$1"
  $cmd --help | bat --language=cmd-help
}

To highlight command --help messages.
@victor-gp
Copy link
Contributor Author

You are both right. I had overlooked that syntaxes are applied according to file extension, and cmd-help already supports that. 😅

I added a test based on bat -h, which hits most syntax elements. I also included help as a file extension related to the syntax, and bumped the submodule here.

As for usage, I suggest in the readme to use this, which covers most UX concerns (ask if you want me to elaborate). Surely it's not a very visible place, much less to the average bat user, so I'm open to suggestions with regards to this.

@sharkdp
Copy link
Owner

sharkdp commented Apr 7, 2022

As for usage, I suggest in the readme to use this, which covers most UX concerns (ask if you want me to elaborate). Surely it's not a very visible place, much less to the average bat user, so I'm open to suggestions with regards to this.

I think we could add a new "Colorization of --help texts" subsection here: https://github.com/sharkdp/bat#integration-with-other-tools (or similar title). I would first mention how to do it without any shell function and alias, and add the two commands that you have in your README.

@victor-gp
Copy link
Contributor Author

Done, see the latest commit. Open to edits :D

README.md Outdated
@@ -212,6 +212,22 @@ Also, note that this will [not work](https://github.com/sharkdp/bat/issues/1145)

The [`prettybat`](https://github.com/eth-p/bat-extras/blob/master/doc/prettybat.md) script is a wrapper that will format code and print it with `bat`.

#### Highlighting `--help` messages

You can use bat to colorize help text: `$ cp --help | bat -plhelp`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other places in the readme, we put bat in backticks.

Suggested change
You can use bat to colorize help text: `$ cp --help | bat -plhelp`
You can use `bat` to colorize help text: `$ cp --help | bat -plhelp`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad! Fixed.

I also included a line to (try to) channel issues my way.

@victor-gp
Copy link
Contributor Author

Hi! Can we move this forward? Is there anything I can do to help?

Copy link
Collaborator

@Enselic Enselic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a quick look and it looks OK to me too. Maybe we should be a bit more terse in the README, because I think our README currently is a bit too long, but we can do that later I think.

Since @sharkdp expressed support for this in the issue itself, I think we can merge this now.

@Enselic Enselic merged commit b089890 into sharkdp:master Apr 26, 2022
@victor-gp
Copy link
Contributor Author

Thank you! :)

I'm cool with the terseness. One way to go about it is making a separate section for "you can also use a wrapper around this..." in the syntax's readme. Then we can link that anchor here and narrow this to 2-3 lines (and no code block). Should I do that?

@Enselic
Copy link
Collaborator

Enselic commented Apr 26, 2022

If you have an idea for how to reduce the size of the readme AND make it easier to read as a whole, then: yes, I think you should do that :) Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants