Skip to content

Handle new visibility types #970

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

Closed
kamalmarhubi opened this issue May 2, 2016 · 4 comments
Closed

Handle new visibility types #970

kamalmarhubi opened this issue May 2, 2016 · 4 comments

Comments

@kamalmarhubi
Copy link
Contributor

syntex_syntax 0.31 brings a change from

pub enum Visibility {
    Public,
    Inherited,
}

to

pub enum Visibility {
    Public,
    Crate,
    Restricted { path: P<Path>, id: NodeId },
    Inherited,
}

It's unclear if the new variants need handling.

kamalmarhubi added a commit to kamalmarhubi/rustfmt that referenced this issue May 2, 2016
Most of the churn on this bump comes from the `Visibility` enum changing
from

    pub enum Visibility {
        Public,
        Inherited,
    }

to

    pub enum Visibility {
        Public,
        Crate,
        Restricted { path: P<Path>, id: NodeId },
        Inherited,
    }

which require taking `Visibility` by reference in most places. The new
variants are not handled at this point.

Refs rust-lang#970
@tikue
Copy link
Contributor

tikue commented May 27, 2016

The following code is actually removed by rustfmt:

pub(super) enum WriteState<D> {
    WriteId {
        id: U64Writer,
        size: U64Writer,
        payload: Option<Writer<D>>,
    },
    WriteSize {
        size: U64Writer,
        payload: Option<Writer<D>>,
    },
    WriteData(Writer<D>),
}

When I plop it in tests/target/pub-restricted.rs and run cargo test the output is:

Mismatch at tests/target/pub-restricted.rs:1:

-pub(super) enum WriteState<D> {⏎
-    WriteId {⏎
-        id: U64Writer,⏎
-        size: U64Writer,⏎
-        payload: Option<Writer<D>>,⏎
-    },⏎
-    WriteSize {⏎
-        size: U64Writer,⏎
-        payload: Option<Writer<D>>,⏎
-    },⏎
-    WriteData(Writer<D>),⏎
-}⏎
+⏎

@kamalmarhubi
Copy link
Contributor Author

This appears to be a fault of how I handled adding them, which is to say, I didn't: kamalmarhubi@fd38acb#diff-3dbd16223f25e8b5e0bcf099743107e9R74

kamalmarhubi added a commit to kamalmarhubi/rustfmt that referenced this issue May 27, 2016
This commit properly handles pub(restricted) as introduced in RFC 1422
[0]. The syntax support was added in rust-lang#971, but they were not correctly
formatted.

[0] https://github.com/rust-lang/rfcs/blob/master/text/1422-pub-restricted.md

Fixes rust-lang#970
@kamalmarhubi
Copy link
Contributor Author

@tikue I opened #1013 to support it properly. Thanks for the test!

@tikue
Copy link
Contributor

tikue commented May 27, 2016

Nice! Quick turnaround😉

marcusklaas pushed a commit that referenced this issue May 27, 2016
* Handle pub(restricted)

This commit properly handles pub(restricted) as introduced in RFC 1422
[0]. The syntax support was added in #971, but they were not correctly
formatted.

[0] https://github.com/rust-lang/rfcs/blob/master/text/1422-pub-restricted.md

Fixes #970

* Drop #[inline] attribute on format_visibility

* Make newly non-failing functions return String

The change to `format_visibiilty` means that `format_header` and
`format_unit_struct` can no longer fail. Their return type is updated to
reflect that.
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

No branches or pull requests

2 participants