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

change case defaults #805

Merged
merged 5 commits into from
Feb 21, 2024
Merged

change case defaults #805

merged 5 commits into from
Feb 21, 2024

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Feb 3, 2024

@Emilgardis
Copy link
Member

I think it would be worth with this to add #[doc(alias = "<peripheral name in svd>")]

@burrbull
Copy link
Member Author

burrbull commented Feb 3, 2024

Looks like starting _/digit sometimes incompatible with PascalCase.

@burrbull burrbull force-pushed the case-defaults branch 2 times, most recently from d836b29 to 59aa45a Compare February 3, 2024 12:58
@burrbull
Copy link
Member Author

burrbull commented Feb 3, 2024

/ci diff pr

@burrbull
Copy link
Member Author

burrbull commented Feb 3, 2024

I don't have ideas what to do with Freescale.

@Emilgardis
Copy link
Member

ugh, why does the /ci triggers fail here, I have no issues with https://github.com/cross-rs/cross/blob/main/.github/workflows/try.yml ...

/ci diff pr

Copy link

github-actions bot commented Feb 3, 2024

Diff for comment

@Emilgardis
Copy link
Member

also, sometimes the github action log messes up whitespace with colors

but, diff looks good, except

     impl RegisterBlock {
         ///0x04..0x14 - SRAM/NOR-Flash chip-select timing register %s
         #[inline(always)]
-        pub const fn btr(&self, n: usize) -> &BTR {
+        pub const fn Btr(&self, n: usize) -> &Btr {
             #[allow(clippy::no_effect)] [(); 4][n];
             unsafe { &*(self as *const Self).cast::<u8>().add(4).add(8 * n).cast() }
         }

should be btr, not Btr

@burrbull
Copy link
Member Author

burrbull commented Feb 3, 2024

ugh, why does the /ci triggers fail here, I have no issues with

Are you sure someone else except you can run this?

@Emilgardis
Copy link
Member

There's nothing in it that would prohibit other members to run it, the only check is that the user making the comment is a member, which we both are

@burrbull
Copy link
Member Author

burrbull commented Feb 3, 2024

/ci diff pr

@Emilgardis
Copy link
Member

    {
      "id": "IC_kwDOBDHJwM5yw0Sf",
      "author": {
        "login": "burrbull"
      },
      "authorAssociation": "MEMBER",

I don't understand...

/ci diff pr

Copy link

github-actions bot commented Feb 3, 2024

Diff for comment

@burrbull burrbull force-pushed the case-defaults branch 5 times, most recently from 756e025 to 3311fa8 Compare February 3, 2024 22:09
@burrbull
Copy link
Member Author

burrbull commented Feb 3, 2024

@Emilgardis run ci yet one time, please.

@Emilgardis
Copy link
Member

can you try it, this time put some text before

@Emilgardis
Copy link
Member

/ci diff pr

Copy link

github-actions bot commented Feb 3, 2024

Diff for comment

@Emilgardis
Copy link
Member

/ci diff --pager "git diff --no-index" pr

Copy link

github-actions bot commented Feb 3, 2024

Diff for comment

@Emilgardis

This comment was marked as outdated.

src/config.rs Outdated Show resolved Hide resolved
@burrbull burrbull marked this pull request as ready for review February 4, 2024 06:35
@burrbull burrbull changed the title change case defaults, fix bugs change case defaults Feb 16, 2024
@Emilgardis Emilgardis added T-tools S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Feb 16, 2024
@burrbull
Copy link
Member Author

I'm not sure myself if that is the best way to go about it, maybe we could have "themes" where there's a default theme (the new behaviour), and another theme that looks like how it used to look like. You can still set individual formats if wanted.
so maybe

ident_formats_theme = "legacy"

Done. Not sure about new theme name.

@burrbull burrbull force-pushed the case-defaults branch 3 times, most recently from 37fd184 to cf31f32 Compare February 16, 2024 20:30
src/config.rs Outdated Show resolved Hide resolved
@Emilgardis
Copy link
Member

Done. Not sure about new theme name.

We could also not have a new theme, e.g not expose it

@Emilgardis
Copy link
Member

maybe default?

With this variant though (legacy and default/new), we get into a issue of versioning, where if we change the theme we'd probably have to introduce a new theme for the previous version. I think only having legacy makes sense for now. Maybe there's an argument for having ranged theme names, such that saying theme = "0.35.1" means you get the appropriate theme for that version, but sounds like hell to maintain and keep track of

@thejpster
Copy link
Contributor

thejpster commented Feb 16, 2024

2021 and 2024? (Or whatever year makes sense)

@burrbull
Copy link
Member Author

@Emilgardis Can you fix svd2rust-regress "unused" warnings?

@burrbull burrbull added this pull request to the merge queue Feb 21, 2024
@Emilgardis Emilgardis removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Feb 21, 2024
Merged via the queue into master with commit d8f1468 Feb 21, 2024
47 checks passed
@burrbull burrbull deleted the case-defaults branch February 21, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UPPER CASE, lower case and Title Case
5 participants