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

Don't use std::unique_ptr for SVEvent #3870

Closed
wants to merge 1 commit into from

Conversation

rmast
Copy link

@rmast rmast commented Jul 17, 2022

@stweil
Copy link
Member

stweil commented Jul 17, 2022

This reverts parts of the commits 58e8538 and 37b3374, but misses some destructor calls which were previously used.

@@ -836,7 +836,7 @@ char ScrollView::Wait() {
char ret = '\0';
SVEventType ev_type = SVET_ANY;
do {
std::unique_ptr<SVEvent> ev(AwaitEvent(SVET_ANY));
SVEvent *ev(AwaitEvent(SVET_ANY));
Copy link
Member

@stweil stweil Jul 17, 2022

Choose a reason for hiding this comment

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

4.0.0 already used std::unique_ptr for this functionality (see code).

@rmast
Copy link
Author

rmast commented Jul 17, 2022 via email

@rmast
Copy link
Author

rmast commented Jul 17, 2022 via email

@stweil
Copy link
Member

stweil commented Jul 17, 2022

For 4.x that code was in callcpp.cpp (see link in my previous comment).

@rmast
Copy link
Author

rmast commented Jul 17, 2022

For 4.x that code was in callcpp.cpp (see link in my previous comment).

Yes. I saw that on second look. My commit primarily reverses your second commit, but hardly the first one. I wonder whether those other variables are also transferred to and from Java, and whether the std:: -wrapping gives trouble with the Java-application.

@amitdo
Copy link
Collaborator

amitdo commented Jul 18, 2022

Replaced by #3872.

@amitdo amitdo closed this Jul 18, 2022
@rmast rmast deleted the issue-3869 branch August 15, 2022 18:26
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