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

feat: compatiable with mihomo icon field #562

Merged
merged 4 commits into from
Sep 1, 2024
Merged

Conversation

greenhat616
Copy link
Collaborator

🤔 This is a ...

  • New feature
  • Bug fix
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

Nope

💡 Background and solution

Mihomo introduce the icon field for showing a custom icon processed by GUI.
Doc link: https://wiki.metacubex.one/config/proxy-groups/?h=icon#icon

What's more, it seems that:

  • we could introduce the enumflags2 for better group match(reducing duplicate code).
  • we could shared the opts and serialize via serde(flatten). The behavior should be similar to the anonymous field of golang. Doc ref: https://serde.rs/attr-flatten.html

📝 Changelog

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is not needed
  • Changelog is not needed

@greenhat616 greenhat616 requested a review from ibigbug August 31, 2024 05:24
@ibigbug
Copy link
Member

ibigbug commented Aug 31, 2024

there is a CommonOptions

pub struct CommonOption {
but only on plain outbound, maybe add it to the groups too?

@greenhat616
Copy link
Collaborator Author

there is a CommonOptions

pub struct CommonOption {

but only on plain outbound, maybe add it to the groups too?

Oh, I missed this opts. I think I could do it later.

Additionaly, I think the handler opts could be merged into a share opts too.

@ibigbug
Copy link
Member

ibigbug commented Aug 31, 2024

some handlers has that too. they are pretty much the same which we can just pass down.

however on groups the connect_via is unsupported, but should be fine

@greenhat616
Copy link
Collaborator Author

Yes. It chould be a huge refactor tweak to cleanup or redesign if possible.

Besides that, The OutboundType could be tweaked later. which approach do you think is better?

  1. use enumflags to create a sub matching group. It just put a derive macro and add extra methods. But it will use up to a u32 size. And, maybe we can use it refactor some config opts?
  2. use a nested type? maybe can use TryInto or other trait to make work?
  3. other approach?

Signed-off-by: Yuwei Ba <contact@yba.dev>
@ibigbug
Copy link
Member

ibigbug commented Aug 31, 2024

yeah i agree. will look into it later, thanks!

re the enumflags2, I haven't thought much about it and I'm not so familia with the crate either - how would that change how the code looks like? do you have an example?

@greenhat616
Copy link
Collaborator Author

greenhat616 commented Aug 31, 2024

yeah i agree. will look into it later, thanks!

re the enumflags2, I haven't thought much about it and I'm not so familia with the crate either - how would that change how the code looks like? do you have an example?

The behavior is similar to the crate bitflags, but is designed for enum.

It impl the equivalence of the iota in golang.

const (
    VERBOSE MyConf = 1 << iota
    CONFIG_FROM_DISK
    DATABASE_REQUIRED
    LOGGER_ACTIVATED
    DEBUG
    FLOAT_SUPPORT
    RECOVERY_MODE
    REBOOT_ON_FAILURE
)

The equivalence logic in enumflags:

use enumflags2::{bitflags, make_bitflags, BitFlags};

#[bitflags]
#[repr(u8)]
#[derive(Copy, Clone, Debug, PartialEq)]
enum Test {
    VERBOSE,
    CONFIG_FROM_DISK,
    DATABASE_REQUIRED,
    LOGGER_ACTIVATED,
    DEBUG,
    FLOAT_SUPPORT,
    RECOVERY_MODE,
    REBOOT_ON_FAILURE,
}

And you can use it do the bitwise ops. And it provides candys.

Here is a usage in Nyanpasu:

use enumflags2::{bitflags, BitFlag, BitFlags};

#[bitflags]
#[repr(u8)]
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq)]
pub enum ClashCore {
    #[serde(rename = "clash", alias = "clash-premium")]
    ClashPremium = 0b0001,
    #[serde(rename = "clash-rs")]
    ClashRs,
    #[serde(rename = "mihomo", alias = "clash-meta")]
    Mihomo,
    #[serde(rename = "mihomo-alpha")]
    MihomoAlpha,
}

/// a function return result:
pub fn builtin() -> Vec<(BitFlags<ClashCore>, ChainItem)> {
// ...

vec![
            (ClashCore::Mihomo | ClashCore::MihomoAlpha, hy_alpn),
            (ClashCore::Mihomo | ClashCore::MihomoAlpha, meta_guard),
            (ClashCore::all(), config_fixer),
            (ClashCore::ClashRs.into(), clash_rs_comp),
]
}

/// the final usage
 for item in ChainItem::builtin()
            .into_iter()
            .filter(|(s, _)| s.contains(*clash_core.as_ref().unwrap_or(&ClashCore::default()))) // This line is the bitflags judgement
            .map(|(_, c)| c)
{
  // ...
}

Ref: https://github.com/libnyanpasu/clash-nyanpasu/blob/f910c600878c281fd40c59c75f84fe6e32f4d968/backend/tauri/src/enhance/chain.rs#L133-L138, https://github.com/libnyanpasu/clash-nyanpasu/blob/f910c600878c281fd40c59c75f84fe6e32f4d968/backend/tauri/src/enhance/mod.rs#L118C8-L121C29

@ibigbug
Copy link
Member

ibigbug commented Aug 31, 2024

i see. thanks for the example. i think it has limited use case here and tbh I think it also adds extra costs to the readability

thoughts? @litcc @VendettaReborn

@VendettaReborn
Copy link
Contributor

yeah, i agree, the benefit of enumflags2 is rather limited

@ibigbug ibigbug merged commit 78ae10a into Watfaq:master Sep 1, 2024
23 checks passed
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.

3 participants