bat icon indicating copy to clipboard operation
bat copied to clipboard

Automatically rebuild custom assets when necessary

Open Enselic opened this issue 3 years ago • 4 comments

Currently, if a user makes use of custom assets and then installs a new 0.x.0 version of bat, bat will stop working. That is not an ideal user experience. It would be good if we could make this use case smoother, e.g. by automatically doing the equivalent of running bat cache --build for the user.

Step-by-step

Here is a step by step against the git repo, but regular users will get this problem too via regular releases from us and e.g. package manager updates of the bat app.

  1. cargo run -- cache --build --blank --source assets
  2. Bump 0.x.0 version in Cargo.toml, e.g. from 0.19.0 to 0.20.0
  3. cargo run -- examples/simple.rs

Expected result

bat prints examples/simple.rs

Actual result

[bat error]: The binary caches for the user-customized syntaxes and themes in
'/Users/martin/.cache/bat' are not compatible with this version of bat (0.20.0). To solve this,
either rebuild the cache (bat cache --build) or remove the custom syntaxes/themes (bat
cache --clear).
For more information, see:

  https://github.com/sharkdp/bat#adding-new-syntaxes--language-definitions

Enselic avatar Feb 23 '22 18:02 Enselic

I did consider this when first implementing the above semver-incompatibility detection and error message. My only concern is that we can not know if the binary user assets and the source user assets are still "compatible". This might be a contrived case, but what I was thinking about is the following scenario:

  • user puts custom assets in $(bat --config-dir)/syntax
  • user runs bat cache --build
  • user somehow modifies files in $(bat --config-dir)/syntax (e.g.: removes source files, or adds some source files but then decides to not run bat cache --build)
  • new version of bat comes along that simply rebuilds the binary assets from source and changes the user behavior.

I therefore opted for the explicit version rather than the implicit/automatic one.

But I'm happy to reconsider this. I might have been a bit over-cautious.

sharkdp avatar Mar 06 '22 18:03 sharkdp

* user _somehow_ modifies files in `$(bat --config-dir)/syntax` (e.g.: removes source files, or adds some source files but then decides to not run `bat cache --build`)

* new version of `bat` comes along that simply rebuilds the binary assets from source and changes the user behavior.

What if bat manages its caches transparently. Users should only have control over the plain configuration files. User behavior is governed by its config files.

awerlang avatar Mar 26 '22 14:03 awerlang

You mean bat should check the timestamp of each custom theme and custom syntax, and then compare that with the timestamp of the built custom assets (a.k.a. "the cache"), and if any asset is newer than the cache, then the cache is automatically rebuilt?

I think that is a good solution. My only concern is what impact if would have on startup time. But we already today read the metadata file on each startup, so I suspect doing something akin to above would not have that big of an impact.

If we can get that to work, we potentially could get rid of the cache subcommand entirely, which would be nice. It would fix #1726 for example.

I think it would also take care of the scenario you describe David, because as soon as the user "breaks" their custom assets, they would learn about it right away. (Or to be precise, the next time they invoke bat.)

Enselic avatar Mar 27 '22 10:03 Enselic

You mean bat should check the timestamp of each custom theme and custom syntax, and then compare that with the timestamp of the built custom assets (a.k.a. "the cache"), and if any asset is newer than the cache, then the cache is automatically rebuilt?

I'm not advocating for any solution in particular here. An actual solution should find a cached asset that semver matches the running bat, and source checksum also matches. If matching fails then rebuild and add or replace cache.

I also linked https://github.com/dandavison/delta/issues/1018 since other programs might depend on bat caches but support a different library version. IMO programs should not be sharing caches this way without a proper solution as outlined above. This is important as Rust doesn't support dynamic linking, in contrast to how C libraries would be packaged. I also appreciate ~/.cache/bat can be empty and it just works.

awerlang avatar Mar 28 '22 19:03 awerlang

I would like to suggest updating the completion scripts to prevent autocompleting the cache subcommand alongside file names. This will eventually be done when this issue is closed, but I believe there is good reason to do it immediately:

Imagine this situation (which gets me almost every day):

  1. I make a new folder and cd into it
  2. I use cp or mv to copy/move in a file, let's call it foo.bar
  3. I want to view foo.bar with bat
  4. Knowing there is only one file in this directory, I type bat , hit tab and enter, expecting the foo.bar to get completed
  5. Nothing got completed because Bash doesn't know whether to complete cache or foo.bar
  6. bat got run, so bat hangs while waiting for input on stdin

You can argue that this is user error on my part, or that my fingers are too fast for my own good (which could be true), but I would insist that the completion script is at fault here. The reason is simply that this behaviour is dissimilar to most other commonly-used tools (e.g. cat, diff, your favourite editor, etc.), and is certainly not what the user expects.

And in return for the constant frustration, the benefit is clearly minimal. The bat-cache subcommand is very rarely used if ever (at least in my experience), so its existence in the completion script is more of a stumbling block than anything else.

Therefore I think it's a good idea to remove it right now from all completion scripts. Should be only a few lines of deletion. If people agree with this opinion then I'm happy to go ahead and create a PR.

P.s. I'm only suggesting the removal of the autocompletion of the word cache, not the removal of the autocompletion of bat-cache's flags. These don't really cause trouble and can stay until the subcommand gets removed entirely.

cyqsimon avatar Oct 07 '22 14:10 cyqsimon

Therefore I think it's a good idea to remove it right now from all completion scripts. Should be only a few lines of deletion. If people agree with this opinion then I'm happy to go ahead and create a PR.

I personally have no objections.

keith-hall avatar Oct 09 '22 20:10 keith-hall

I would like to suggest updating the completion scripts to prevent autocompleting the cache subcommand alongside file names.

big +1, this would be a huge improvement in usability – thank you!

acrewdson avatar Oct 11 '22 23:10 acrewdson