Errors for bat cache --build when cache already exists and BAT_CACHE_PATH is specified

[zachriggle]

Describe the bug you encountered:

I am attempting to distribute a custom bat theme with my tool to use. I want to avoid the possibility of messing with the user's installed bat themes and theme cache.

Notably, the bat cache --build --blank command does not work when the destination path already exists, and it throws an unusual error message.

Note that it works the first time:

$ mkdir -p /tmp/bat-cache-bug

$ cd /tmp/bat-cache-bug

$ XDG_CONFIG_HOME=$PWD BAT_CACHE_PATH=$PWD/cache bat cache --build
No themes were found in '/tmp/bat-cache-bug/bat/themes', using the default set
No syntaxes were found in '/tmp/bat-cache-bug/bat/syntaxes', using the default set.
Writing theme set to /tmp/bat-cache-bug/cache/themes.bin ... okay
Writing syntax set to /tmp/bat-cache-bug/cache/syntaxes.bin ... okay
Writing metadata to folder /tmp/bat-cache-bug/cache ... okay

But fails the second time, with the exact same command:

$ XDG_CONFIG_HOME=$PWD BAT_CACHE_PATH=$PWD/cache bat cache --build
error: Found argument '--build' which wasn't expected, or isn't valid in this context

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

For more information try --help

What did you expect to happen instead?

Rebuilding the cache would also be nice, as that's the expected behavior.

A better error message would be helpful.

How did you install bat?

brew install bat

---

bat version and environment

$ XDG_CONFIG_HOME=$PWD BAT_CACHE_PATH=$PWD/cache bat --diagnostic

Software version

bat 0.18.1 ()

Operating system

macOS 12.0 (Darwin 21.0.0)

Command-line

bat --diagnostic

Environment variables

SHELL=/bin/zsh
PAGER=less
LESS='-F -i -M -R -S -w -X -z-4'
BAT_PAGER=<not set>
BAT_CACHE_PATH=/tmp/bat-cache-bug/cache
BAT_CONFIG_PATH=<not set>
BAT_OPTS=<not set>
BAT_STYLE=plain
BAT_TABS=<not set>
BAT_THEME=<not set>
XDG_CONFIG_HOME=/tmp/bat-cache-bug
XDG_CACHE_HOME=<not set>
COLORTERM=truecolor
NO_COLOR=<not set>
MANPAGER='sh -c '\''col -bx | bat -l man -p'\'''

Config file

Could not read contents of '/tmp/bat-cache-bug/bat/config': No such file or directory (os error 2).

Compile time information

• Profile: release
• Target triple: x86_64-apple-darwin
• Family: unix
• OS: macos
• Architecture: x86_64
• Pointer width: 64
• Endian: little
• CPU features: fxsr,sse,sse2,sse3,ssse3
• Host: x86_64-apple-darwin

Less version

> less --version
less 487 (POSIX regular expressions)
Copyright (C) 1984-2016  Mark Nudelman

less comes with NO WARRANTY, to the extent permitted by law.
For information about the terms of redistribution,
see the file named README in the less distribution.
Homepage: http://www.greenwoodsoftware.com/less

[sharkdp]

Thank you for reporting this. I believe this is due to the fact that (after the first run) there is a subdirectory called cache in that folder. When calling bat cache --build the second time, bat interprets the cache argument as the directory name.

We have seen a few variations of this bug in the past (#584, #557, #245). I thought we fixed them all. But this is new. I guess we only looked for files called cache so far, not directories. This should be fixed.

[zachriggle]

So bat is attempting to see which arguments are a file, before parsing them as subcommands?

[sharkdp]

So bat is attempting to see which arguments are a file, before parsing them as subcommands?

Well, there is only one subcommand that bat has (bat cache). In hindsight, it was a mistake to design it as a subcommand. A separate binary (bat-cache) or a command-line option (bat --build-cache / bat --clear-cache) would have been the better choice. Why? Because cache can be a completely legal file name*. So if there is a file called cache in the current directory, we assume that the user wanted to display that file (instead of issuing a cache-modifying command). We also do this for POSIX-compliance reasons (compatibility with cat).

Looks like we hadn't thought about the case when there's a directory called cache. I think we could actually assume that, in this case, the user actually wants to issue the cache subcommand.

* even worse. Due to the way clap (the command line parsing library we use) works, partial matches are also possibly. So we also had problems with files called ca and/or cache.c.

[sharkdp]

I seem to remember that @eth-p wanted to remove the cache subcommand quite early. That would have been the right call. Maybe it still would be - despite the breakage. What do the other maintainers think? (cc @keith-hall @Enselic)

[Enselic]

Since the cache subcommand keeps giving us problems, I am in favour of getting rid of it. I vote for something akin to bat --build-cache / bat --clear-cache over a separate binary, simply because it is much easier to maintain a single binary.

Before we take a final decision it would be useful to know how much special-case code we can get rid of if we make this move. Probably quite a bit? Maybe someone can throw together a draft PR for that?

Speaking of breakage, it currently looks like it will be hard to solve #951 without introducing some breakage, so maybe it would make sense to collect these (and maybe even more breaking changes) in a "breaking changes" release.

[keith-hall]

Since the cache subcommand keeps giving us problems, I am in favour of getting rid of it. I vote for something akin to bat --build-cache / bat --clear-cache over a separate binary, simply because it is much easier to maintain a single binary.

I'm 110% on board with this ;)

[sharkdp]

Alright. I'm also okay with this, if someone can detail how we design the new CLI. The bat cache subcommand has quite a few additional options that would need to be moved to bat somehow (without confusing users):

▶ bat cache -h
bat-cache 
Modify the syntax-definition and theme cache

USAGE:
    bat cache [OPTIONS] <--build|--clear>

OPTIONS:
    -b, --build           Initialize (or update) the syntax/theme cache.
    -c, --clear           Remove the cached syntax definitions and themes.
        --source <dir>    Use a different directory to load syntaxes and themes from.
        --target <dir>    Use a different directory to store the cached syntax and
                          theme set.
        --blank           Create completely new syntax and theme sets (instead of
                          appending to the default sets).
    -h, --help            Prints help information

[Enselic]

Here are some proposals. They all take into account that we would like to change the term cache to custom assets. (We recently added a --no-custom-assets flag, for example.) In all proposals, we would use the normal help:

bat --help

Proposal A. Similar to old CLI

# build, default args
bat --custom-assets --build

# build, all args
bat --custom-assets --build --source=/a/b --blank --target=/c/d

# clear
bat --custom-assets --clear

Proposal B. All args namespaced, and --custom-assets is implicit

# build, default args
bat --custom-assets-build

# build, all args
bat --custom-assets-build --custom-assets-source=/a/b --custom-assets-blank --custom-assets-target=/c/d

# clear
bat --custom-assets-clear

Proposal C. Same as B, but without separate source
Instead, we let --custom-assets-build accept an optional source path. That allows us to get rid of one more arg.

# build, default args
bat --custom-assets-build

# build, all args
bat --custom-assets-build=/a/b --custom-assets-blank --custom-assets-target=/c/d

# clear
bat --custom-assets-clear

Proposal D. Same as C, but with different wording

# build, default args
bat --build-custom-assets

# build, all args
bat --build-custom-assets=/a/b --build-without-integrated-assets --custom-assets-dest-dir=c/d

# clear
bat --clear-custom-assets

Proposal E. Structured like high-level and low-level commands
High level workflow, as described in README.md, for regular users.
There is no way to exclude integrated assets with these commands.
Integrated assets are always included.

# build custom assets from config dir and put in cache dir
bat --build-custom-assets
# clear custom assets
bat --clear-custom-assets

Low level commands, for maintainers and advanced users.
Integrated assets are never included.

# Build assets
bat --build-assets=/source/dir:/target/dir

My current personal favorites is something akin to E.

[sharkdp]

I also like proposal E. Thank you for writing this down.

[domWalters]

Hi,

I've found another annoying way that the cache command can trip you up.

I had the bat command aliased to bat --tabs 2 (before anyone says so, yes I have since realised that --tabs can be set in the config file).

So when I ran bat cache --build it was invisibly running bat --tabs 2 cache --build.

This throws an error as you might expect:

error: unexpected argument '--build' found

  tip: to pass '--build' as a value, use '-- --build'

Usage: bat --theme <theme> --style <components> --tabs <T> <FILE>...

For more information, try '--help'.

Using bat --diagnostic, I eventually realised what the problem was. Migrating to using --tabs in the config file avoided the issue.