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

Partially sync Regedit to Wine-7.17 #4717

Merged
merged 3 commits into from Nov 2, 2022

Conversation

gonzoMD
Copy link
Member

@gonzoMD gonzoMD commented Sep 21, 2022

Purpose

Partially sync Regedit to Wine-7.17

JIRA issue: Don't know.

Proposed changes

[REGEDIT] Partially Sync to Wine-7.17

  • regproc.c and regedit.c are now in sync.
  • some other mostly depending fixes for the remaining files

[REGEDIT_WINETEST] Sync to Wine-7.0

Before:

VirtualBox_ReactOS_06_09_2022_12_44_26

After:

VirtualBox_ReactOS_21_09_2022_21_27_35

- regproc.c and regedit.c are now in sync.
- some other mostly depending fixes for the remaining files
@github-actions github-actions bot added the ROSTESTS Label for ROS testcases PRs. label Sep 21, 2022
@binarymaster binarymaster added enhancement For PRs with an enhancement/new feature. 3rd party sync Updating 3rd party components, such as Wine and others labels Sep 21, 2022
@JoachimHenze
Copy link
Contributor

"Maybe" CORE-8141? Why not testing it?

@gonzoMD
Copy link
Member Author

gonzoMD commented Sep 22, 2022

"Maybe" CORE-8141? Why not testing it?

I did. It doesn't fix the error:
VirtualBox_ReactOS_22_09_2022_18_53_01

I removed the mention of the ticket.

base/applications/regedit/regedit.c Show resolved Hide resolved
}
void WINAPIV output_message(unsigned int id, ...)
{
WCHAR fmt[1536];
Copy link
Member

Choose a reason for hiding this comment

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

.....
This is the readme text size?

Copy link
Member Author

Choose a reason for hiding this comment

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

I hope so. :D I didn't measure the string.

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand the code, the worst-case-impact on too long translations, would be cutting off the string.
Which is not optimal, but most likely tolerable. Better than corruption at least.
Do you agree @learn-more that we are safe from corruption?

base/applications/regedit/regedit.c Show resolved Hide resolved
Copy link
Contributor

@JoachimHenze JoachimHenze left a comment

Choose a reason for hiding this comment

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

Imho the many fixed tests do justify that PR.
A lot of work for updating the translations will follow most likely.
Only thing that I see could be improved is the fixed size of the helpstring. That might give problems, if translators would chose very long translations maybe.

@JoachimHenze
Copy link
Contributor

gonzoMD you touched the tests as well. The old untouched tests for regedit were 100% successful on Win2k3sp2. see
https://reactos.org/testman/compare.php?ids=84199
Can you prove please with a screenshot, that this is also still the case for the updated tests?

@gonzoMD
Copy link
Member Author

gonzoMD commented Oct 14, 2022

Can you prove please with a screenshot, that this is also still the case for the updated tests?

Next week, I'll be back to my PC where I have my 2k3 VM. I'll do then. Maybe I get the stuff set up to my other PC earlier too.

@binarymaster
Copy link
Member

Can you prove please with a screenshot, that this is also still the case for the updated tests?

regedit: 3080 tests executed (0 marked as todo, 0 failures), 0 skipped.

Tested on XP SP3 and Win2k3 R2 SP2, result is the same.

xp sp3

win2k3 r2 sp2

@gonzoMD
Copy link
Member Author

gonzoMD commented Oct 14, 2022

regedit: 3080 tests executed (0 marked as todo, 0 failures), 0 skipped.

Tested on XP SP3 and Win2k3 R2 SP2, result is the same.

Sbasiba

Copy link
Member

@binarymaster binarymaster left a comment

Choose a reason for hiding this comment

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

Ship it 🚢

@gonzoMD gonzoMD merged commit 36a7f0d into reactos:master Nov 2, 2022
@gonzoMD gonzoMD deleted the try_to_winesync_regedit branch November 2, 2022 18:02
JoachimHenze pushed a commit that referenced this pull request Nov 4, 2022
* [REGEDIT] Partially Sync to Wine 7.17

- regproc.c and regedit.c are now in sync.
- some other mostly depending fixes for the remaining files

* [REGEDIT_WINETEST] Sync to Wine-7.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party sync Updating 3rd party components, such as Wine and others enhancement For PRs with an enhancement/new feature. ROSTESTS Label for ROS testcases PRs.
Projects
None yet
4 participants