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

Add SSH config syntax #582

Merged
merged 1 commit into from
Jun 7, 2019
Merged

Conversation

issmirnov
Copy link
Contributor

Tested:

bash assets/create.sh
cargo build
target/debug/bat --language ssh_config ~/.ssh/config

Tested:
```
bash assets/create.sh
cargo build
target/debug/bat --language ssh_config ~/.ssh/config
```
@sharkdp
Copy link
Owner

sharkdp commented Jun 7, 2019

Thank you very much!

It would be great if we could somehow make this work automatically for ~/.ssh/config. Obviously, we don't want to enable this for every file named config, but maybe there is a good way to add new functionality to bat to support this.

@sharkdp sharkdp merged commit 175dd4c into sharkdp:master Jun 7, 2019
@issmirnov
Copy link
Contributor Author

Thanks! Happy to help.

Regarding detection of files, one idea I've been playing with is adding a first_line_match directive (see here). This opens a few possibilities.

  1. Users annotate files manually. It's fine for SSH config files since generally there's only a few of them on each system. First line should be # -*- mode: ssh-config -*-
  2. The first line match is expanded to guess if it's an SSH conf file. Ie, ^IdentityFile or ^Host\s(\w*) match, or some other smart regexes.

If we go that route though, that will likely require a patch to the sublime-syntax file, just like the Java one currently.

Thoughts?

@sharkdp
Copy link
Owner

sharkdp commented Jun 8, 2019

Regarding detection of files, one idea I've been playing with is adding a first_line_match directive (see here).

That's definitely a good first step and something that we could implement before we have a more general solution.

I was thinking it would be even better if we could provide a "out of the box" solution for every user of bat. I believe we could achieve something like this if we would add a "Path => Syntax" mapping database somewhere within bat. This would work similar to the "File extension => Syntax" and "File name => Syntax" mappings already present in the Sublime Text syntax files.

For the SSH config, we could then explicitly check if the (expanded) file path matches $HOME/.ssh/config.

The first line match is expanded to guess if it's an SSH conf file. Ie, ^IdentityFile or ^Host\s(\w*) match, or some other smart regexes.

Hmm.. I think that could lead to false positive matches. I'd rather not follow that route.

[...] will likely require a patch to the sublime-syntax file, just like the Java one currently.

I'm not very happy with the hack I did for the Java syntax. I would try to avoid having more patched files like this.

To add the first_line_match directive, we could

  • Fork the original repository and depend on that in bat.
  • Think about adding a feature in bat which would add such a # -*- mode: XXX -*- first-line-match directive automatically for every syntax.

What do you think?

@issmirnov
Copy link
Contributor Author

Awesome suggestions!

add a "Path => Syntax" mapping database

I think this is great. This would work for hosts, ssh config, and likely a whole number of files that are simply called config or hosts, etc.

first-line-match directive automatically for every syntax.

If it's not too hard to implement, this would bring bat in line with vim/emacs as far as file type identification capabilities. In order to not restart the age old wars, probably best to support both # -*- mode: XXX -*- and # vim:ft=XXX. Full disclosure, not a emacs user so would be good to double check the syntax.

@sharkdp
Copy link
Owner

sharkdp commented Jun 11, 2019

I'm going to add open two new feature requests in separate issues. Thank you very much for your feedback!

@sharkdp
Copy link
Owner

sharkdp commented Aug 31, 2019

Released in bat v0.12.

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.

2 participants