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

installer: avoid cut-off text #536

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

dscho
Copy link
Member

@dscho dscho commented Dec 7, 2023

As reported in git-for-windows/git#4727, with Windows' switch of the default system font a couple labels are now cut off. Let's fix this by re-wrapping the text accordingly.

@dscho dscho self-assigned this Dec 7, 2023
@dscho
Copy link
Member Author

dscho commented Dec 7, 2023

This affects the Editor, DefaultBranch, Path and BashTerminal pages:

image

image

image

image

@rimrul
Copy link
Member

rimrul commented Dec 8, 2023

Windows' switch of the default system font a couple labels are now cut off.

Are we sure that's the root cause? The reported issues don't reproduce for me on Windows 11 at 100% ui scaling and 100% text scaling. I suspect this might be a ui scaling or text scaling issue where this fix would Only fix things for this user, but not others with more severe scaling.

@dscho
Copy link
Member Author

dscho commented Dec 8, 2023

Windows' switch of the default system font a couple labels are now cut off.

Are we sure that's the root cause?

Hmm. Maybe not. I thought that was the only thing that changed on my machine since I last looked over (and fixed) cut-off text.

FWIW I also experimented with code like this:

diff --git a/installer/install.iss b/installer/install.iss
index d97adfab1..031914ab9 100644
--- a/installer/install.iss
+++ b/installer/install.iss
@@ -1356,6 +1356,89 @@ begin
     end;
 end;
 
+procedure CheckParagraphForRewrapping(Text:String;Width:Integer;Font:TFont);
+var
+    S,Rewrapped:String;
+    next,space,LineWidth:Integer;
+begin
+    S:=Text;
+    StringChangeEx(S,#13,' ',True);
+    while True do begin
+        if next>=Length(S) then begin
+            if S<>'' then begin
+//LogError('Left-over S: '+S);
+                if Rewrapped<>'' then
+                    Rewrapped:=Rewrapped+#13;
+                Rewrapped:=Rewrapped+S;
+            end;
+//LogError('Rewrapped: '+Rewrapped);
+            if Text<>Rewrapped then begin
+                StringChangeEx(Text,#13,'+#13+',True);
+                StringChangeEx(Rewrapped,#13,'+#13+',True);
+                LogError('Rewrap required (width '+IntToStr(Width)+'):'+#13+Text+#13+'->'+#13+Rewrapped);
+            end;
+            Exit;
+        end;
+
+        space:=next+1;
+        while (space<=Length(S)) and (S[space]<>#32) do
+            space:=space+1;
+//LogError('Width of: '+SubString(S,1,space-1)+' is: '+IntToStr(GetTextWidth(SubString(S,1,space-1),Font)));
+        LineWidth:=GetTextWidth(SubString(S,1,space-1),Font);
+        if LineWidth<=Width then begin
+            next:=space;
+        end else begin
+LogError('Sub: '+SubString(S,1,space-1)+' LineWidth:'+IntToStr(LineWidth)+', Width: '+IntToStr(Width));
+            if (next=0) then begin
+                LogError('Word is too wide ('+SubString(S,1,space-1)+')!');
+                Exit;
+            end;
+            if Rewrapped<>'' then
+                Rewrapped:=Rewrapped+#13;
+            Rewrapped:=Rewrapped+SubString(S,1,next-1);
+            if next<Length(S) then
+                Delete(S,1,next)
+            else
+                S:='';
+            next:=0;
+            space:=0;
+//LogError('T: '+S+#13+'Sub: '+SubString(S,1,space-1)+#13+'Rewrapped: '+Rewrapped);
+        end;
+    end;
+end;
+
+procedure CheckWhetherTextNeedsToBeRewrapped(Text:String;Width:Integer;Font:TFont);
+var
+    i,eop:Integer;
+begin
+#if 0
+    while True do begin
+        i:=Pos(''+#13,Text);
+        if i=0 then begin
+            if GetTextWidth(Text,Font)>Width then
+                LogError('Too long line: '+Text);
+            Exit;
+        end else begin
+            if GetTextWidth(SubString(Text,1,i-1),Font)>Width then
+                LogError('Too long line: '+SubString(Text,1,i-1));
+            Delete(Text,1,i);
+        end;
+    end;
+#else
+    StringChangeEx(Text,#13+'(WARNING',#13+#13+'(WARNING',True);
+    while True do begin
+        eop:=Pos(''+#13+#13,Text);
+        if eop=0 then begin
+            CheckParagraphForRewrapping(Text,Width,Font);
+            Exit;
+        end;
+
+        CheckParagraphForRewrapping(SubString(Text,1,eop-1),Width,Font);
+        Delete(Text,1,eop+1);
+    end;
+#endif
+end;
+
 function CountLines(S:String):Integer;
 begin
     Result:=1+StringChangeEx(S,#13,'',True);
@@ -1390,6 +1473,9 @@ begin
             '': begin
                 Untagged:=Untagged+Description;
                 Result.Caption:=Untagged;
+#if APP_VERSION=='0-test2'
+                CheckWhetherTextNeedsToBeRewrapped(Untagged,Result.Width,Result.Font);
+#endif
                 Result.Height:=ScaleY(13*RowCount);
                 Top:=Top+13+18;
                 Exit;

It ended up being a complete bust: the Result.Width turned out to be highly variable, even if it had originally been assigned the value ScaleX(405), and as a consequence, the new functions would report completely bogus suggestions that either led to cut-off text or to wide empty margins on the right side.

As reported in git-for-windows/git#4727,
currently the installer shows labels that are cut off. It is a bit
unclear what caused this: While I first suspected the new Windows font,
Aptos, to be the culprit, that hypothesis was rejected when another
contributor reported that the text appears not to be cut off for them
(git-for-windows#536 (comment)).

Let's address this by re-wrapping the text more conservatively, so that
the text appears intact in more setups.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the fix-cut-off-text-in-installer branch from 72ff42d to 053916a Compare December 8, 2023 17:27
@dscho
Copy link
Member Author

dscho commented Dec 8, 2023

@rimrul I reworded the commit message to make a less outrageous claim. Here's the range-diff:

  • 1: 72ff42d ! 1: 053916a installer: avoid cut-off text

    @@ Metadata
      ## Commit message ##
         installer: avoid cut-off text
     
    -    As reported in https://github.com/git-for-windows/git/issues/4727, with
    -    Windows' switch of the default system font a couple labels are now cut
    -    off. Let's fix this by re-wrapping the text accordingly.
    +    As reported in https://github.com/git-for-windows/git/issues/4727,
    +    currently the installer shows labels that are cut off. It is a bit
    +    unclear what caused this: While I first suspected the new Windows font,
    +    Aptos, to be the culprit, that hypothesis was rejected when another
    +    contributor reported that the text appears not to be cut off for them
    +    (https://github.com/git-for-windows/build-extra/pull/536#issuecomment-1847227487).
    +
    +    Let's address this by re-wrapping the text more conservatively, so that
    +    the text appears intact in more setups.
     
         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
     

@dscho dscho merged commit f3ceb6e into git-for-windows:main Dec 13, 2023
6 checks passed
@dscho dscho deleted the fix-cut-off-text-in-installer branch December 13, 2023 10:04
github-actions bot pushed a commit that referenced this pull request Dec 13, 2023
The Git for Windows installer [showed cut-off text in some
setups](git-for-windows/git#4727). This [has
been fixed](#536).

Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>
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