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

bat as a library #423

Closed
mre opened this issue Nov 19, 2018 · 22 comments · Fixed by #679
Closed

bat as a library #423

mre opened this issue Nov 19, 2018 · 22 comments · Fixed by #679
Labels
feature-request New feature or request help wanted Extra attention is needed
Milestone

Comments

@mre
Copy link

mre commented Nov 19, 2018

Recently I needed a way to pretty-print some Rust code to the command-line while building cargo-inspect.
I used syntect, but in a very crude way. It occured to me, that by using bat as a pretty-printer, I could piggy-back on what already exists and get features like line-numbers for free.
Probably other projects would want to do the same.

So I wonder if you considered providing a lib.rs, which would allow for using this crate as a library.
The main.rs, would then simply use the lib.rs as a dependency. That seems to be quite a common pattern. As an additional advantage, it would decouple the argument parsing from the rest of the code. Would that be possible?

@sharkdp
Copy link
Owner

sharkdp commented Nov 21, 2018

Thank you for the feedback.

I am not planning to do that. I'm also not sure if the resulting library would really be a useful library as it mixes together too many things, in my opinion. There is syntax highlighting, git modifications, lots of direct I/O operations, line numbering, different styles, themes, caching-logic, customization-logic, integration with the pager, wrapping, etc. etc.

If we want to build a useful library with a clean API out of this, we would have to separate it into many pieces and you would probably be left with something like syntect (?).

@mre
Copy link
Author

mre commented Nov 21, 2018

Yeah I'm using syntect right now, but it just doesn't cut it. The things you mentioned are sorely missing - and for good reason: a library that focuses on one thing is very powerful.
That said, I'd love to have all that functionality in a convenient (meta-) package. I'm aware that this is outside of the scope of bat itself, but I really like all the features this project provides and it would be great to have a shared codebase somehow.

Alternatively, an output-agnostic pretty-printer on top of syntect might be an option. It could evolve independently from bat. I could try that, but I'm a bit afraid of the maintenance costs. If anybody knows of such a project, I'm all ears.

@sharkdp sharkdp added the question Further information is requested label Nov 25, 2018
@mre
Copy link
Author

mre commented Nov 29, 2018

Just whipped up a quick crate for that. Say hi to prettyprint 👋 . I don't expect much resonance from that, as I mostly need it for cargo-inspect and didn't want to bake it in there.

License is the same as for bat. I gave credit to bat in the docs, but in case I should make that more prominent, feel free to create a PR.

So, I guess we can close this issue. 😊

@sharkdp
Copy link
Owner

sharkdp commented Dec 2, 2018

Just whipped up a quick crate for that. Say hi to prettyprint . I don't expect much resonance from that, as I mostly need it for cargo-inspect and didn't want to bake it in there.

It will be a bit complicated to stay up to date with what is happening in bat, but that sounds like a good compromise for now.

I can now see that you really want most of bats functionality bundled up in a library, so if you think that there is a clean way to integrate the library/executable split within this repository, I'd be open to bundling everything up within the bat crate here. By "clean", I mean that we wouldn't have to add lots of boilerplate/duplicated code.

License is the same as for bat. I gave credit to bat in the docs, but in case I should make that more prominent, feel free to create a PR.

Thank you! The attribution looks good. To be formally compliant with the open source licences, I think you would have to add a copy of bats version of the LICENSE in your repo or as a header in the copied source files. I have added a Copyright notice here: https://github.com/sharkdp/bat/blob/master/LICENSE-MIT. Maybe we should actually add a APACHE-copyright header in each of bats source files here.

@mre
Copy link
Author

mre commented Dec 2, 2018

so if you think that there is a clean way to integrate the library/executable split within this repository, I'd be open to bundling everything up within the bat crate here. By "clean", I mean that we wouldn't have to add lots of boilerplate/duplicated code.

In guess the easiest way would be to use prettyprint as a library in bat. Sounds weird at first, but if you think about it, this allows for a clear separation of concerns: a crate to handle formatting and a standalone binary to use it. bat could simply use the builder in prettybuild to generate output. We could make prettyprint a part of the bat umbrella in the long run, but maybe as part of a separate Github organization, e.g. github.com/bat-rs, which is currently free. I would keep both in separate repos. This would allow the library to be easily discovered by people like me who need the same functionality. 😊

The caveat is, that I changed most &str types to String types along the way, because that was quicker to get working ownership-wise, so we might have to change that back.

I think you would have to add a copy of bats version of the LICENSE in your repo

Actually, there is. 😉

I've added your copyright header as well. 👍

I've added the copyright notice

@sharkdp
Copy link
Owner

sharkdp commented Dec 7, 2018

In guess the easiest way would be to use prettyprint as a library in bat.

I'm sorry, but that's not a very attractive option for me. This would complicate working on bat a lot. Contributors would have to integrate their features into prettyprint first and open a second PR in bat to add the gluing-code or the version bump. It would also complicate the release process for me.

Actually, there is.

I've added your copyright header as well.

Oh great, thank you very much!

@quodlibetor
Copy link

@sharkdp if you wanted to move most of bat into a library you could:

  • create a new project inside this directory cargo new --lib batlib (or whatever)

  • move most of main.rs into a lib.rs file in batlib/src/lib.rs, and most of the files in bat into that src dir. I typically try to have only a main.rs and an args.rs in my main binary crates if I can, but most of my cli scripts are pretty tightly entwined with their library code.

  • decide which parts of the lib.rs are necessary to be public for bat's and pretty-print's code to function and add pub to them -- prettyprint's API is pretty minimal for the actual printing part.

  • modify Cargo.toml to include a workspace with the only member being batlib, rust book docs, something like this should be pretty close:

    [workspace]
    members = [ "batlib" ]
    

then cargo publish will probably do the right thing and cargo test --all should test everything.

@mitsuhiko
Copy link

I would also love this to be available as a library. I tried prettyprint but it misses too much right now and contributing to the fork seems suboptimal. I like the idea that @quodlibetor brought up of just making a batlib. If that would be acceptable I could provide a patch that enables that.

@sharkdp
Copy link
Owner

sharkdp commented Jan 16, 2019

@quodlibetor Thank you very much for your suggestions!

@mitsuhiko Thank you for the feedback. I think I have changed my mind (with respect to my first comment here). I'm happy to support splitting bat into a libbat (I guess that would be more natural than batlib) and a thin CLI wrapper around it. If you would like to work on this, I'd be very glad to take PRs!

Just to be sure, I'd like to list a few things that I would require (read: I would need to hear very good reasons if we want to break with one of these) if we go forward with this:

  • Most of the code stays in the main bat repository. I'm generally okay with splitting out smaller parts of the code into a well-designed crate (and different repo), but I currently don't really see a subset of features that would make a very good standalone library (apart from libbat itself).
  • The actual release process will not be complicated (too much).
  • The CI process and the automated deployment via TravisCI/Appveyor still works as expected.

Concerning the "release process" part: If I understand @quodlibetor correctly, there will be just one (public) crate called bat with a single version and libbat would just follow the same versioning?

@quodlibetor
Copy link

@sharkdp

there will be just one (public) crate called bat with a single version and libbat would just follow the same versioning?

Not quite: there would just be one git repository https://github.com/sharkdp/bat but the directory structure would include:

.
├── Cargo.toml
├── libbat
│  ├── Cargo.toml
│  └── src
│     └── lib.rs
└── src
   └── main.rs

libbat would be independently versionable, and the dependencies list in the ./Cargo.toml would just include libbat = { version = "...", path = "./libbat" } -- cargo automatically prefers a path dependency over crates.io, but requires versions to still match (I believe).

I don't recall what the release procedure becomes -- it might be just cd libbat && cargo publish && cd .. cargo publish if bat's version changes require changes to libbat, but in general you get to decide what you publish, and how.

@mitsuhiko
Copy link

Most of the code stays in the main bat repository. I'm generally okay with splitting out smaller parts of the code into a well-designed crate (and different repo), but I currently don't really see a subset of features that would make a very good standalone library (apart from libbat itself).

I don't think splitting into a separate repo is necessary.

The actual release process will not be complicated (too much). […] Concerning the "release process" part: If I understand @quodlibetor correctly, there will be just one (public) crate called bat with a single version and libbat would just follow the same versioning?

There would be two crates but version in lockstep out of the same repo. I have typically a small script I run to help make the release (bumps the versions in the cargo toml).

I will make a fork and send a PR with some initial work for this.

@mitsuhiko
Copy link

Also it can be done less invasive than @quodlibetor suggests in terms of directory layout. I will try to approaches.

@quodlibetor
Copy link

Oh I'm excited to see that.

@mitsuhiko
Copy link

mitsuhiko commented Jan 16, 2019

I started an initial attempt in #469. There is a bit of work needed to reshape things into a library (batwing) and the CLI (bat) but it seems pretty simple. The main things I ran into so far:

  • controller uses the PROJECT_DIR variable. This one is probably not desirable for batwing
  • i was lazy so i added a cli feature that implements the entire CLI in batwing for now that bat depends on. bat is just a mini executable that invokes the cli in batwing
  • I exported pretty much all the API other than the controller and CLI from the library for now.
  • I ended up with two directories for now because i think it will be cleaner long term to move batwing and bat apart a bit.

@mre maybe you want to have a look at if what I did here makes any sense since you already attempted something similar for prettyprint.

@codesections
Copy link

Just wanted to +1 how great this feature would be.

I just released mnemonic, which relies on pretyprint to print simple notes files to the terminal—and it would be great to be able to rely on batwing directly.

Thanks for changing your mind about implementing batwing (and, as others have said, great name!)

@Walther
Copy link

Walther commented Mar 17, 2019

One additional reason why I think the librarification is amazing: this enables people to build CLI applications with visual cohesion, as the same coloring logic can be used across various tools. Additionally, all of those other programs can get theming support for free - and the themes will be consistent!

@vosscodes
Copy link

vosscodes commented Jul 8, 2019

edit: kind of overwhelmed between day job and contract right now unfortunately

@sharkdp sharkdp added feature-request New feature or request and removed question Further information is requested labels Oct 11, 2019
@sharkdp sharkdp reopened this Oct 20, 2019
@sharkdp
Copy link
Owner

sharkdp commented Oct 20, 2019

Re-opening this, as there are a few more to-do's. See #693

@sharkdp
Copy link
Owner

sharkdp commented Oct 30, 2019

Moving the to do list here:

  • Move the dirs module to the "binary" part of the crate. This involves a restructuring of the assets module. For the library, it would first be enough to only provide the cached assets (HighlightingAssets::new()).
  • Possibly move the error-handling to the binary (the handle_error function). We don't want the library to print [bat error]: …. Instead, we could pass a callback to the Controller.
  • The public API interface is huge at the moment. We should try to minimize this by making things private. Otherwise, we will have lots of (technically) API-breaking changes in the future.

@sharkdp sharkdp added this to the v0.13 milestone Mar 6, 2020
@sharkdp sharkdp changed the title Library support? bat as a library Mar 21, 2020
@sharkdp
Copy link
Owner

sharkdp commented Mar 21, 2020

With #873 and #875 merged, I think this ticket can finally be closed. The next release (see #845) will include a first version of bat as a library.

@sharkdp sharkdp closed this as completed Mar 21, 2020
@sharkdp
Copy link
Owner

sharkdp commented Mar 22, 2020

Please see the v0.13 release notes for further instructions: https://github.com/sharkdp/bat/releases/tag/v0.13.0

@hacker-DOM
Copy link

Hey folks, I remember there was an issue to create a custom PAGER. Check out https://github.com/noborus/ov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants