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

Make bat::PrettyPrinter::syntaxes() iterate over new bat::Syntax struct #2222

Merged
merged 1 commit into from
Sep 3, 2022

Conversation

Enselic
Copy link
Collaborator

@Enselic Enselic commented Jun 19, 2022

We can't keep syntect::parsing::SyntaxReference as part of the public API, because that might prevent us from bumping to syntect 6.0.0 without also bumping bat to v2.0.0.

So introduce a new stripped down struct Syntax and return that instead. Let it be fully owned to make the API simple. It is not going to be in a hot code path anyway.

I have looked at all code of our 27 dependents but I can't find a single instance of this method being used, so this change should be safe for v1.0.0. It is only used in our own example code as far as I can tell.

For reference (mostly to my future self), here is the script I used to download the code of all dependents. Then one can grep around in the code of dependents:

#/usr/bin/env bash

# curl -L 'https://crates.io/api/v1/crates/bat/reverse_dependencies' | jq --raw-output '.versions | map(.crate) | flatten[]'
# curl -L 'https://crates.io/api/v1/crates/bat/reverse_dependencies?page=2' | jq --raw-output '.versions | map(.crate) | flatten[]'
# curl -L 'https://crates.io/api/v1/crates/bat/reverse_dependencies?page=3' | jq --raw-output '.versions | map(.crate) | flatten[]'
bat_dependents="
barberousse
borrowing_exerci
bropages
cargo-expand
download_caretaker
dsconv
dts
etrade
fix-hidden-lifetime-bug-proc_macros
gistit-cli
gistit
git-delta-lib
git-delta
gooseberry
hgrep
longboard
mdn
next-gen-proc_macros
nu_plugin_textview
partiql-rs
piqel
pmis
qcow-cli
raen
riffle
trrs
with_locals-proc_macros
"

for crate in $bat_dependents; do
    echo "Processing $crate"
    mkdir -p "$crate"
    cd "$crate"
    newest_version=$(curl -L https://crates.io/api/v1/crates/$crate | jq --raw-output .crate.newest_version)
    echo "$newest_version"
    curl -L "https://crates.io/api/v1/crates/$crate/$newest_version/download" | tar -zxf -
    cd ..

    sleep 2 # avoid throttling
done

Also see #2221

@Enselic
Copy link
Collaborator Author

Enselic commented Jun 21, 2022

Just to be clear

  • We should release this (and other 1.0.0 RC changes) as the usual bat 0.x.0 bump, i.e. we should not go directly to 1.0.0
  • I will update CHANGELOG.md before merge

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much. This looks good to me!

…struct

We can't keep `syntect::parsing::SyntaxReference` as part of the public
API, because that might prevent us from bumping to syntect 6.0.0 without
also bumping bat to v2.0.0, once we reach v1.0.0.

So introduce a new stripped down struct `Syntax` and return that
instead. Let it be fully owned to make the API simple. It is not going
to be in a hot code path anyway.

I have looked at all code of our 27 dependents but I can't find a single
instance of this method being used, so this change should be safe for
v1.0.0.
@Enselic Enselic changed the title PrettyPrinter::syntaxes(): Prepare for v1.0.0 Make bat::PrettyPrinter::syntaxes() iterate over new bat::Syntax struct Sep 3, 2022
@Enselic
Copy link
Collaborator Author

Enselic commented Sep 3, 2022

For the record, here is the API diff from this PR:

% cargo public-api --diff-git-checkouts origin/master stabilize-syntaxes
Removed items from the public API
=================================
(none)

Changed items in the public API
===============================
-pub fn bat::PrettyPrinter::syntaxes(&self) -> impl Iterator<Item = &SyntaxReference>
+pub fn bat::PrettyPrinter::syntaxes(&self) -> impl Iterator<Item = Syntax> + '_

Added items to the public API
=============================
+#[non_exhaustive] pub struct bat::Syntax
+pub struct field bat::Syntax::file_extensions: Vec<String>
+pub struct field bat::Syntax::name: String

@Enselic Enselic merged commit 49875d6 into sharkdp:master Sep 3, 2022
@Enselic Enselic deleted the stabilize-syntaxes branch September 3, 2022 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants