-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
proposal to support raw_attribute with raw pointer #124180
base: master
Are you sure you want to change the base?
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ChrisDenton (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
This comment has been minimized.
This comment has been minimized.
7b4d550
to
3d88f2b
Compare
This comment has been minimized.
This comment has been minimized.
3d88f2b
to
9de7584
Compare
☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts. |
8352a3e
to
064f71c
Compare
☔ The latest upstream changes (presumably #130865) made this pull request unmergeable. Please resolve the merge conflicts. |
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged. You can start a rebase with the following commands:
The following commits are merge commits: |
This comment has been minimized.
This comment has been minimized.
@cre4ture any updates on the CI failure? thanks |
@Dylan-DPC thanks for the trigger. |
2954fc2
to
1f4ba49
Compare
Hello rust community,
I'm still rather new to rust, but I collected experience with rust in the uutils project.
There I'm currently trying to make use of the conpty windows feature and stumbled across the same topic as discussion in #114854.
This was my solution to it and I found that its probably simpler as all other solutions I've seen on the discussion topic (#121951, #123604). And thus its easier to understand, use and therefor might take less discussions to get it approved.
The key element is, that I don't change the existing approach for normal data attributes with the Box::new(). I just extend the API with a second variant that takes raw pointers.
Just let me know what you think.
If there is positive feedback, I will adapt documentation and tests.
I also added a change (force_use_std_handles) that will be needed to have a proper support of the PSEUDOCONSOLE / conpty.
Its clear that this should be in a different PR. I will split it as needed after some feedback.