Skip to content

Enhance Tool Macro Functionality #161

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

yIllusionSky
Copy link

Changes

  • Removed rust-toolchain.toml file as project doesn't need to restrict to a specific Rust version
  • Enhanced tool macro functionality:
    • Automatically recognize #[tool(param)] and #[tool(aggr)] parameters without manual marking
    • Added tool_impl_item functionality for default implementation of ServerHandler trait
    • Added default_build parameter (defaults to true, can be disabled with default_build=false)
  • Fixed reference to local libraries instead of Git repositories in Cargo.toml
  • Fixed issue with impl not recognizing doc attributes
  • Updated calculator and other examples to demonstrate new features
  • Modified related test cases

Benefits

These improvements simplify API usage and reduce manual configuration needs. In most cases, users only need to mark #[tool(tool_box)] to get default implementation of the ServerHandler trait.

Testing

Basic tests have been performed to verify functionality.

@4t145
Copy link
Collaborator

4t145 commented May 7, 2025

Apart from your functional changes, I want to point out follows:

  1. Do not upload binary file, which is .DS_store in this case, add you are supposed to add it into .gitignore file.
  2. There are a lot of changes like converting format!("{a}") to format!("{a}"). If it's introduced by format tools, you are supposed to change the format config. And if not, could you explain the motivation to do so.
  3. Do not remove rust-toolchain.toml file, this project has a MSRV of 1.85. It can remind those people who are still using earlier version.
  4. As for commit messages, 如果你不想在提交记录里面留下你的真名,你可以为公开仓库单独设置用户名。
  5. You may ignored my question in Is rmcp macro too complicated? #159, do you have a answer on it?

@yIllusionSky
Copy link
Author

Apart from your functional changes, I want to point out follows:

  1. Do not upload binary file, which is .DS_store in this case, add you are supposed to add it into .gitignore file.
  2. There are a lot of changes like converting format!("{a}") to format!("{a}"). If it's introduced by format tools, you are supposed to change the format config. And if not, could you explain the motivation to do so.
  3. Do not remove rust-toolchain.toml file, this project has a MSRV of 1.85. It can remind those people who are still using earlier version.
  4. As for commit messages, 如果你不想在提交记录里面留下你的真名,你可以为公开仓库单独设置用户名。
  5. You may ignored my question in Is rmcp macro too complicated? #159, do you have a answer on it?

Got it.
ds_store was an accident. I didn't pay attention to clear them, and Mac automatically generated them.

As for the problem of comment names, it is related to my git config configuration. I have always been working on personal projects and trying to submit PR for the first time, so I don't have enough experience in these.

Some insignificant changes are related to my use of cargo clippy --fix after deleting rustfmt. Some things may have been changed by clippy (guessing)

@4t145
Copy link
Collaborator

4t145 commented May 7, 2025

@yIllusionSky Yeah, It's your freedom to leave your real name in commit message, just a reminder since I can tell that you don't often commit to public project.

And we may need a clippy config @jokemanfire. It would be another pr.

illusion_autumn added 2 commits May 8, 2025 03:28
## Changes

Simplified the usage of rcmp-macros with the following changes:

### Before
```rust
#[tool(tool_box)]
impl Calculator {
    #[tool(description = "Calculate the sum of two numbers")]
    fn sum(&self, #[tool(aggr)] SumRequest { a, b }: SumRequest) -> String {
        (a + b).to_string()
    }

    #[tool(description = "Calculate the difference of two numbers")]
    fn sub(
        &self,
        #[tool(param)]
        #[schemars(description = "the left hand side number")]
        a: i32,
        #[tool(param)]
        #[schemars(description = "the right hand side number")]
        b: i32,
    ) -> Json<i32> {
        Json(a - b)
    }
}
```

### After
```rust
#[tool(tool_box)]
impl Calculator {
    #[tool(description = "Calculate the sum of two numbers",aggr)]
    fn sum(&self, SumRequest { a, b }: SumRequest) -> String {
        (a + b).to_string()
    }

    #[tool(description = "Calculate the difference of two numbers")]
    fn sub(
        &self,
        #[schemars(description = "the left hand side number")]
        a: i32,
        #[schemars(description = "the right hand side number")]
        b: i32,
    ) -> Json<i32> {
        Json(a - b)
    }
}
```

## Improvements
1. Moved parameter-level `#[tool(aggr)]` attribute to the function level
2. Removed redundant `#[tool(param)]` markers
3. Maintained the same functionality while making the code more concise and readable
## Changes

This modification simplifies the usage of rmcp-macros, with the following key changes:

### 1. Merged Toolbox Declaration and Description

**Reference Example:**

Original code:
```rust
#[tool(tool_box)]
impl Calculator {
    // Tool method implementations
}

#[tool(tool_box)]
impl ServerHandler for Calculator {
    fn get_info(&self) -> ServerInfo {
        ServerInfo {
            instructions: Some("A simple calculator".into()),
            capabilities: ServerCapabilities::builder().enable_tools().build(),
            ..Default::default()
        }
    }
}
```

Simplified:
```rust
#[tool(tool_box,description = "A simple calculator")]
impl Calculator {
    // Tool method implementations
}
```

### 2. Simplified Implementation Process

- No longer requires separate implementation of the `ServerHandler` trait
- Tool descriptions are provided directly through macro parameters
- Reduces redundant code, improving maintainability

## Advantages

1. More concise code
2. Reduced boilerplate
3. More intuitive API usage
4. Lower barrier to entry
@yIllusionSky
Copy link
Author

@yIllusionSky Yeah, It's your freedom to leave your real name in commit message, just a reminder since I can tell that you don't often commit to public project.

And we may need a clippy config @jokemanfire. It would be another pr.

I reconstructed a new one according to the idea I gave you, except for a few differences in the naming.

@jokemanfire
Copy link
Collaborator

jokemanfire commented May 8, 2025

And we may need a clippy config @jokemanfire. It would be another pr.我们可能需要一个 clippy 配置。这将是另一个 pull request。

right, I am interested in this submit and I will also take a look later.

@jokemanfire jokemanfire changed the title Enhance Tool Macro Functionality and Remove Rust-Toolchain Enhance Tool Macro Functionality May 8, 2025
@yIllusionSky
Copy link
Author

And we may need a clippy config @jokemanfire. It would be another pr.我们可能需要一个 clippy 配置。这将是另一个 pull request。

right, I am interested in this submit and I will also take a look later.

I tried to fix the cli's warn error.
In addition, I found some other problems, as follows

Warning: can't set `imports_granularity = Crate`, unstable features are only available in nightly channel.
Warning: can't set `group_imports = StdExternalCrate`, unstable features are only available in nightly channel.
Warning: can't set `unstable_features = true`, unstable features are only available in nightly channel.

These rustfmt configurations require unstable configuration to start, but rust-toolchain does not configure unstable

@jokemanfire
Copy link
Collaborator

jokemanfire commented May 9, 2025

you may forget update the stable channel in rustc. :)

@yIllusionSky
Copy link
Author

you may forget update the stable channel in rustc. :)

I have been developing with the latest nightly version of rust, and I update rustup almost every week.I think that might not be the problem?

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