Skip to content

Replace paste by with_builtin_macros.#1394

Merged
maximebuyse merged 2 commits intomainfrom
remove-paste-dependency
Apr 9, 2025
Merged

Replace paste by with_builtin_macros.#1394
maximebuyse merged 2 commits intomainfrom
remove-paste-dependency

Conversation

@maximebuyse
Copy link
Copy Markdown
Contributor

Fixes #1379.

I looked for a solution without introducing a new dependency. concat_idents seems like a good candidate but it doesn't work for function names as discussed in rust-lang/rust#29599. So I replaced paste by with_builtin_macros which builts on top of concat_idents to provide the functionality we need.

@maximebuyse maximebuyse requested a review from a team as a code owner April 8, 2025 14:52
@maximebuyse maximebuyse requested a review from W95Psp April 8, 2025 14:54
Copy link
Copy Markdown
Contributor

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

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

That looks good to me, let's see how maintained with_builtin_macros is, but for now that seems a great solution! :)

And that works without nightly, so that's nice.

@Nadrieril
Copy link
Copy Markdown
Collaborator

Are you aware of ${concat(x, y)}?

Copy link
Copy Markdown
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

with_builtin_macros doesn't look great 😬
Did you try the macro_metavar_expr_concat? If that works, I think it would be the better option.

@maximebuyse
Copy link
Copy Markdown
Contributor Author

Are you aware of ${concat(x, y)}?

Looks like it does what we need, thanks! If we are ok to use something experimental, I'll try it.

@karthikbhargavan
Copy link
Copy Markdown
Contributor

Not that I am expert, but in the same thread that talks about macro_metavar_expr_concat and ${concat}, the last suggestion, especially for stable rust, appears to be with_builtin_macros?

rust-lang/rust#124225 (comment)

@maximebuyse
Copy link
Copy Markdown
Contributor Author

Not that I am expert, but in the same thread that talks about macro_metavar_expr_concat and ${concat}, the last suggestion, especially for stable rust, appears to be with_builtin_macros?

rust-lang/rust#124225 (comment)

The suggestion comes from the author of with_builtin_macros though.

Copy link
Copy Markdown
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

If we are ok to use something experimental, I'll try it.

If it works, I think something experimental here within the language feels better than a 3rd party crate. We're on nightly anyway.

@maximebuyse
Copy link
Copy Markdown
Contributor Author

If we are ok to use something experimental, I'll try it.

If it works, I think something experimental here within the language feels better than a 3rd party crate. We're on nightly anyway.

Yes it works, let's merge then!

@maximebuyse maximebuyse added this pull request to the merge queue Apr 9, 2025
Merged via the queue into main with commit ae537b5 Apr 9, 2025
16 checks passed
@maximebuyse maximebuyse deleted the remove-paste-dependency branch April 9, 2025 08:23
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.

Crate paste is unmaintained

5 participants