Skip to content

Prepping for Release v1.0.7#90

Merged
tig merged 2 commits intogui-cs:developfrom
tig:v1_0_6
Dec 4, 2022
Merged

Prepping for Release v1.0.7#90
tig merged 2 commits intogui-cs:developfrom
tig:v1_0_6

Conversation

@tig
Copy link
Collaborator

@tig tig commented Dec 4, 2022

No description provided.

@tig tig requested a review from BDisp December 4, 2022 15:39
@tig tig merged commit 92b5d7f into gui-cs:develop Dec 4, 2022
@tig
Copy link
Collaborator Author

tig commented Dec 5, 2022

@BDisp did you actually test the latest NStack (1.0.6/1.0.7) on the latest Terminal.Gui develop branch?

After publishing v1.0.7, and updating Terminal.Gui to use it, the unit tests are failing.

I'm going to unpublish v1.0.7 for now.

@tig tig changed the title Prepping for Release v1.0.6 Prepping for Release v1.0.7 Dec 5, 2022
@BDisp
Copy link
Collaborator

BDisp commented Dec 5, 2022

Yes I tried now and it's failing because of this:

public uint HotKeyTagMask { get; set; } = 0x100000;

Since the range from 0x10000...0x10FFFF is marked as wide characters, spaces are added after the hot keys. It's better waiting for a fix for this first.

@tig
Copy link
Collaborator Author

tig commented Dec 5, 2022

Yes I tried now and it's failing because of this:

public uint HotKeyTagMask { get; set; } = 0x100000;

Since the range from 0x10000...0x10FFFF is marked as wide characters, spaces are added after the hot keys. It's better waiting for a fix for this first.

Please explain more thoroughly. Why did your testing when you approved this PR not fail?

@BDisp
Copy link
Collaborator

BDisp commented Dec 5, 2022

Please explain more thoroughly. Why did your testing when you approved this PR not fail?

My bad really sorry. I only test it for surrogate pairs and didn't realized that it will failed with how the hotkey tag mask handle it.

@BDisp
Copy link
Collaborator

BDisp commented Dec 5, 2022

I think the better way to fix this is by removing the HotKeyTagMask property and deal directly with the HotKey property. What do you think?

@BDisp
Copy link
Collaborator

BDisp commented Dec 5, 2022

Since we need to remove the hotkey if it's other than uppercase for text handling, maybe change the uint HotKeyTagMask property to int HotKeyPos property to save the hotkey position when the text is been handling for wrap, alignment or whatever and using the position to draw the hotkey properly. Well I think this will be the better option.

Edit:
There is already a HotKeyPos property so I'll try to use only it and remove the HotKeyTagMask.

@BDisp
Copy link
Collaborator

BDisp commented Dec 5, 2022

I already fixed it. I'll submit a PR.

@BDisp
Copy link
Collaborator

BDisp commented Dec 5, 2022

@tig I already submitted the PR gui-cs/Terminal.Gui#2247 with the fix. Can you republish the nuget package again, please. Without that the fix won't work.

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.

2 participants