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

Adaptive numbers style based on length/pagination #1334

Closed
xeruf opened this issue Oct 23, 2020 · 5 comments
Closed

Adaptive numbers style based on length/pagination #1334

xeruf opened this issue Oct 23, 2020 · 5 comments
Labels
question Further information is requested

Comments

@xeruf
Copy link
Contributor

xeruf commented Oct 23, 2020

Hey, I am really enjoying bat for peeking at everything, but here is one little pet peeve I recently encountered:
I only want line numbers when the auto-pagination sets in. Beforehand, the files are so small that it doesn't matter, and having no numbers makes copying much easier. But in the pager, they are super useful!

I tried to create a workaround by using bat as a pager for itself, but something doesn't seem to work here:

❯ bat --style header --pager "bat --style numbers" "/tmp"
There is no style option ("less --help" for help)
numbers: No such file or directory
@xeruf xeruf added the feature-request New feature or request label Oct 23, 2020
@sharkdp
Copy link
Owner

sharkdp commented Oct 24, 2020

I tried to create a workaround by using bat as a pager for itself, but something doesn't seem to work here:

Ahh, we disallow usage of bat as a pager in order to avoid infinite recursion. It seems like the implementation is currently less than ideal and also buggy. Instead of printing a warning ("You can not use bat as a pager for bat…"), we silently replace bat by less and keep all command-line arguments (which doesn't make sense, because they are obviously not applicable to less). This should be fixed.

As for your use case: bat always calls the pager (unless specifically configured otherwise), even for small files. The pager then decides to quit itself if the file is small enough. At that point, we can not control whether or not to show line-numbers anymore. I think this would have to be solved in a separate wrapper script.

@sharkdp sharkdp added bug Something isn't working good first issue Good for newcomers and removed feature-request New feature or request labels Oct 24, 2020
@xeruf
Copy link
Contributor Author

xeruf commented Oct 24, 2020

Oh wait, less does that? Uh.
But what does --pagination auto (which seems to be the default) even do then?

       --paging <when>

              Specify  when  to use the pager. To disable the pager, use '--paging=never' or its alias, -P. To disable the pager permanently, set BAT_PAGER to an empty string. To con‐
              trol which pager is used, see the '--pager' option. Possible values: *auto*, never, always.

@xeruf
Copy link
Contributor Author

xeruf commented Oct 24, 2020

I think this would have to be solved in a separate wrapper script.

So I have to first cat the file, measures its length (wc -l) and then pipe it into bat with the appropriate arguments? Considering that I also want the bat header style, this doesn't sound great tbh - I'd have to access the file at least twice in what I came up with.

adrian-rivera added a commit to adrian-rivera/bat that referenced this issue Oct 25, 2020
As mentioned on sharkdp#1334 `bat` should not be used as a value for `pager`,
this change checks both the balue of `bat` provided as a parameter or
as an environment variable.
adrian-rivera added a commit to adrian-rivera/bat that referenced this issue Oct 26, 2020
As mentioned on sharkdp#1334 `bat` should not be used as a value for `pager`,
this change checks both the balue of `bat` provided as a parameter or
as an environment variable.
adrian-rivera added a commit to adrian-rivera/bat that referenced this issue Oct 26, 2020
As mentioned on sharkdp#1334 `bat` should not be used as a value for `pager`,
this change checks both the balue of `bat` provided as a parameter or
as an environment variable.
adrian-rivera added a commit to adrian-rivera/bat that referenced this issue Oct 27, 2020
As mentioned on sharkdp#1334 `bat` should not be used as a value for `pager`,
this change checks both the balue of `bat` provided as a parameter or
as an environment variable.
sharkdp pushed a commit that referenced this issue Oct 29, 2020
As mentioned on #1334 `bat` should not be used as a value for `pager`,
this change checks both the balue of `bat` provided as a parameter or
as an environment variable.
@sharkdp sharkdp added question Further information is requested and removed bug Something isn't working good first issue Good for newcomers labels Oct 30, 2020
@sharkdp
Copy link
Owner

sharkdp commented Oct 30, 2020

Removing the "bug" label, as #1343 has been merged.

So I have to first cat the file, measures its length (wc -l) and then pipe it into bat with the appropriate arguments? Considering that I also want the bat header style, this doesn't sound great tbh - I'd have to access the file at least twice in what I came up with.

It's not great, no. But I'm not really sure we can (or want) to add support for this in bat directly. bat works in a line-oriented way and does not buffer results. This means that it can work in a streaming context (e.g. some-program | bat -l json). So we can not really decide to switch to another style once we have printed more than $screen_height lines.

@sharkdp
Copy link
Owner

sharkdp commented Dec 29, 2020

I'm closing this for now. Please feel free to comment in case it should be re-opened.

@sharkdp sharkdp closed this as completed Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants