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

Still having issues with filenames starting with "cache" #584

Closed
rotty opened this issue Jun 7, 2019 · 12 comments
Closed

Still having issues with filenames starting with "cache" #584

rotty opened this issue Jun 7, 2019 · 12 comments
Labels
bug Something isn't working

Comments

@rotty
Copy link

rotty commented Jun 7, 2019

Using current master (4ee0b2e), there still seems to be an issue that looks like #557:

% cargo run -- cache.c
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/bat cache.c`
error: The subcommand 'cache.c' wasn't recognized
	Did you mean 'cache'?

If you believe you received this message in error, try re-running with 'bat -- cache.c'

USAGE:
    bat [OPTIONS] [FILE]...
    bat <SUBCOMMAND>

For more information try --help

Giving -- doesn't give a workaround:

% ./target/debug/bat -- cache.c
error: The subcommand 'cache.c' wasn't recognized
	Did you mean 'cache'?
[...]
@sharkdp
Copy link
Owner

sharkdp commented Jun 7, 2019

Thank you for reporting this.

I can not reproduce this:

 ▶ echo "int main(){}" > cache.c
 ▶ git checkout 4ee0b2e
 ▶ cargo run -- cache.c
[...]
     Running `target/debug/bat cache.c`
───────┬──────────────────────────────────────────────────
       │ File: cache.c
───────┼──────────────────────────────────────────────────
   1   │ int main(){}
───────┴──────────────────────────────────────────────────
 ▶ rm cache.c
 ▶ cargo run -- cache.c
[...]
[bat error]: 'cache.c': No such file or directory (os error 2)

@rotty
Copy link
Author

rotty commented Jun 7, 2019

Strange; I just now installed Rust via rustup on a separate account on my machine, to check that it's not something in my environment, and bat also failed like initially reported, both for v0.11.0 and current master.

@sharkdp
Copy link
Owner

sharkdp commented Jun 7, 2019

Oh dear, I was fooled by this again. This is actually described in #557. Everything worked fine for me because I had a bat config file (which also supplies some command line arguments). Without the config file, I can reproduce this.

I realize now that it was a bad decision to use subcommands and positional arguments at the same time. If someone has ideas for a nice command-line interface that replaces bat cache, I'd be glad to change this. Maybe we should just ship a second executable (bat-cache)?

Regarding this specific instance of the bug.. I will look into it.

@sharkdp sharkdp added the bug Something isn't working label Jun 7, 2019
@zzamboni
Copy link

zzamboni commented Aug 23, 2019

Hi,

First of all, thanks for bat! It's a delightful utility.

I'm just ran into this problem, however this is with a file named calc, which is somehow interpreted as a subcommand. Prepending the filename with ./ solves the problem:

[~/m/bin]─> bat calc
error: The subcommand 'calc' wasn't recognized
	Did you mean 'cache'?

If you believe you received this message in error, try re-running with 'bat -- calc'

USAGE:
    bat [OPTIONS] [FILE]...
    bat <SUBCOMMAND>

For more information try --help
[~/m/bin]─> bat ./calc
───────┬─────────────────────────────────────
       │ File: ./calc
───────┼─────────────────────────────────────
   1   │ #!/bin/sh
   2   │ echo "scale=5; $@" | bc
───────┴─────────────────────────────────────

About your question: maybe get rid of subcommands alltogether, and convert cache to a new option (bat --cache)?

@sharkdp
Copy link
Owner

sharkdp commented Aug 24, 2019

First of all, thanks for bat! It's a delightful utility.

Thank you for the feedback!

About your question: maybe get rid of subcommands alltogether, and convert cache to a new option (bat --cache)?

Yes, we should get rid of the subcommand. The question is how we design the new command-line interface. There are multiple commands:

bat cache --build
bat cache --build --source some/path
bat cache --build --target other/path
bat cache --build --source some/path --target other/path
bat cache --build --source some/path --target other/path --blank
bat cache --clear
# ...

would all of these become bat --cache --build ... or bat --cache --clear? That doesn't look like a very good choice to me.

@zzamboni
Copy link

Aha, good point. Maybe having a separate command altogether like you suggested (bat-cache) would make sense. It's anyway something that is only occasionally used.

@eth-p
Copy link
Collaborator

eth-p commented Aug 24, 2019

If I might suggest bat-config cache as the command?

The name would be future proof and also provide an avenue for moving some of the informational options like --cache-dir and --config-dir out of the main executable.

You could also do things such as bat-config edit to open the config file in $EDITOR as a convenience option too.

@sharkdp
Copy link
Owner

sharkdp commented Aug 28, 2019

@eth-p I like it 👍

Another thing that could be included would be bat-config generate (see #573)

@sharkdp
Copy link
Owner

sharkdp commented Aug 28, 2019

Thinking a bit more about this.... there's also things I don't like (whether it's bat-cache or bat-config). For now, the bat executable is a standalone package that includes almost everything, like syntaxes, themes, a default configuration, etc. (excluding documentation and shell completions). It just works. If we introduce a second binary, the distribution and packaging process will be a little bit more involved.

Also, having a separate xxx-config command is not something I have seen in other command line tools (?).

Maybe there are any other solutions which would prevent a second binary?

@rotty
Copy link
Author

rotty commented Aug 31, 2019

Also, having a separate xxx-config command is not something I have seen in other command line tools (?).

I think the general pattern is not unheard of; for instance the monotone VCS provides mtn and mtnopt, the latter providing access to config settings to shell scripts, apt has apt-config, ...

However, I'd suggest considering another name instead of bat-config, but something more general, e.g. bat-maint

  • It seems the scope of the extra binary is broader than just config handling. I'd expect bat-config to allow configuring bat, alike git config. bat-config cache --clear reads a bit awkwardly, as it's not related to configuration.
  • foo-config is often used for build scripts (e.g. guile-config, curl-config, ...), which might be slightly confusing. This is not a very strong reason though, IMO.

Just my 2 cents.

Maybe there are any other solutions which would prevent a second binary?

One traditional Unix approach comes to mind, which, strictly speaking, avoids a second binary, but I'm not sure whether it is portable to Windows:

Have the bat binary, before invoking option parsing, inspect argv[0], and when called as bat-config (or whatever), provide a completely different CLI. Under Unix, there can then be a symlink named bat-config pointing to bat executable. However, this would also not be supported by cargo install, I guess.

@sharkdp
Copy link
Owner

sharkdp commented Aug 31, 2019

@rotty: Thank you for the feedback.

Just now, I found a way to fix this (particular instance of this) bug. There are now a few test cases that make sure that we can always print files named cache, cache.c and a test that we do not accidentally print a file named cache if bat cach is called.

I guess we'll give this another try as long as another corner case comes up 😄

@sharkdp
Copy link
Owner

sharkdp commented Aug 31, 2019

Fixed in bat v0.12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants