fix shortening of file names (fixes #3943)

sumatrapdfreader · Dec 28, 2023 · 952e4aa · 952e4aa
1 parent 3f83c1f
commit 952e4aa
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 49 deletions.
diff --git a/src/Menu.cpp b/src/Menu.cpp
@@ -1003,42 +1003,41 @@ static bool __cmdIdInList(UINT_PTR cmdId, UINT_PTR* idsList, int n) {
 
 #define cmdIdInList(name) __cmdIdInList(md.idOrSubmenu, name, dimof(name))
 
-// TODO: write it in a way that handles unicode
+// shorten a string to maxLen characters, adding ellipsis in the middle
+// ascii version that doesn't handle UTF-8
 static TempStr ShortenStringTemp(char* s, int maxLen) {
-    int nRunes = utf8StrLen((u8*)s);
-    if (nRunes <= maxLen) {
-        return s;
-    }
-    size_t menuStrLen = str::Len(s);
-    if (menuStrLen <= maxLen) {
+    size_t sLen = str::Len(s);
+    if (sLen <= maxLen) {
         return s;
     }
-    char* ret = AllocArrayTemp<char>(maxLen);
+    char* ret = AllocArrayTemp<char>(maxLen + 2);
     const size_t half = maxLen / 2;
-    const size_t strSize = menuStrLen + 1; // size()+1 because wcslen() doesn't include \0
-    // Copy first N/2 characters, move last N/2 characters to the halfway point
+    const size_t strSize = sLen + 1; // +1 for terminating \0
+    // copy first N/2 characters, move last N/2 characters to the halfway point
     for (size_t i = 0; i < half; i++) {
         ret[i] = s[i];
         ret[i + half] = s[strSize - half + i];
     }
-    // Add ellipsis
+    // add ellipsis in the middle
     ret[half - 2] = ret[half - 1] = ret[half] = '.';
-    // Ensure null-terminated string
-    ret[maxLen - 1] = '\0';
     return ret;
 }
 
+// shorten a string to maxLen characters, adding ellipsis in the middle
+// works correctly with utf8 strings
 static TempStr ShortenStringUtf8Temp(char* s, int maxRunes) {
     int nRunes = utf8StrLen((u8*)s);
     if (nRunes < 0) {
+        // not a valid utf8
         return ShortenStringTemp(s, maxRunes);
     }
     if (nRunes <= maxRunes) {
         return s;
     }
-    int toRemove = nRunes - maxRunes - 3; // 3 for "..."
+    int toRemove = (nRunes - maxRunes) + 3; // 3 for "..."
     int removeStartingAt = (nRunes / 2) - (toRemove / 2);
-    char* ret = AllocArrayTemp<char>(maxRunes * 4 + 1); // * 4 over-allocates but should be always safe
+    // over-allocate the result by 4x to be always safe
+    char* ret = AllocArrayTemp<char>(maxRunes * 4 + 1);
     char* tmp = ret;
     int n;
     for (int i = 0; i < nRunes; i++) {
@@ -1080,11 +1079,9 @@ static void AddFileMenuItem(HMENU menuFile, const char* filePath, int index) {
     }
 
     TempStr menuString = path::GetBaseNameTemp(filePath);
-    // If the name is too long, save only the ends glued together
-    // E.g. 'Very Long PDF Name (3).pdf' -> 'Very Long...e (3).pdf'
-    const size_t MAX_LEN = 5;
-     // 70;
-    menuString = ShortenStringUtf8Temp(menuString, MAX_LEN);
+    // shorten very long file names so that menu isn't too wide
+    const size_t kMaxRunes = 70;
+    menuString = ShortenStringUtf8Temp(menuString, kMaxRunes);
 
     TempStr fileName = MenuToSafeStringTemp(menuString);
     int menuIdx = (int)((index + 1) % 10);
@@ -1112,7 +1109,7 @@ static void AppendRecentFilesToMenu(HMENU m) {
         }
         AddFileMenuItem(m, fp, i);
     }
-#if 1
+#if 0
     AddFileMenuItem(
         m,
         "\xf0\x9f\xa4\xa3\xf0\x9f\x98\x8a\xf0\x9f\x98\x82\xe2\x9d\xa4\xf0\x9f\x98\x8d\xf0\x9f\x98\x92\xf0\x9f\x91\x8c"

diff --git a/src/utils/StrUtil.cpp b/src/utils/StrUtil.cpp
@@ -134,11 +134,16 @@ static bool isLegalUTF8(const u8* src, int length) {
 
 /* --------------------------------------------------------------------- */
 
+inline int utf8CharLen(const u8* s) {
+    int n = trailingBytesForUTF8[*s] + 1;
+    return n;
+}
+
 /*
  * return true if a UTF-8 sequence is legal
  */
 bool isLegalUTF8Sequence(const u8* source, const u8* sourceEnd) {
-    int n = trailingBytesForUTF8[*source] + 1;
+    int n = utf8CharLen(source);
     if (source + n > sourceEnd) {
         return false;
     }
@@ -149,21 +154,23 @@ bool isLegalUTF8Sequence(const u8* source, const u8* sourceEnd) {
  * return true if UTF-8 string is legal.
  */
 bool isLegalUTF8String(const u8** source, const u8* sourceEnd) {
-    while (*source != sourceEnd) {
-        int n = trailingBytesForUTF8[**source] + 1;
-        if (n > sourceEnd - *source || !isLegalUTF8(*source, n)) {
+    const u8* s = *source;
+    while (s != sourceEnd) {
+        int n = utf8CharLen(s);
+        if (n > sourceEnd - s || !isLegalUTF8(s, n)) {
             return false;
         }
-        *source += n;
+        s += n;
     }
+    *source = s;
     return true;
 }
 
 // return -1 if not a valid utf8 string
 int utf8StrLen(const u8* s) {
     int len = 0;
     while (*s) {
-        int n = trailingBytesForUTF8[*s] + 1;
+        int n = utf8CharLen(s);
         if (!isLegalUTF8(s, n)) {
             return -1;
         }
@@ -173,11 +180,6 @@ int utf8StrLen(const u8* s) {
     return len;
 }
 
-int utf8CharLen(const u8* s) {
-    int n = trailingBytesForUTF8[*s] + 1;
-    return n;
-}
-
 // --- end of Unicode, Inc. utf8 code
 
 bool IsEqual(const ByteSlice& d1, const ByteSlice& d2) {
@@ -1468,10 +1470,13 @@ char& Str::Last() const {
 // without duplicate allocation. Note: since Vec over-allocates, this
 // is likely to use more memory than strictly necessary, but in most cases
 // it doesn't matter
-char* Str::StealData() {
+char* Str::StealData(Allocator* a) {
+    if (a == nullptr) {
+        a = this->allocator;
+    }
     char* res = els;
     if (els == buf) {
-        res = (char*)Allocator::MemDup(allocator, buf, len + kPadding);
+        res = (char*)Allocator::MemDup(a, buf, len + kPadding);
     }
     els = buf;
     Reset();

diff --git a/src/utils/StrUtil.h b/src/utils/StrUtil.h
@@ -301,7 +301,7 @@ struct Str {
     char RemoveAt(size_t idx, size_t count = 1);
     char RemoveLast();
     char& Last() const;
-    char* StealData();
+    char* StealData(Allocator* a = nullptr);
     char* LendData() const;
     bool Contains(const char* s, size_t sLen = 0);
     bool IsEmpty() const;

diff --git a/src/utils/TempAllocator.cpp b/src/utils/TempAllocator.cpp
@@ -70,27 +70,40 @@ TempStr ReplaceTemp(const char* s, const char* toReplace, const char* replaceWit
         return nullptr;
     }
 
+    const char* curr = s;
+    const char* end = str::Find(curr, toReplace);
+    if (!end) {
+        // optimization: nothing to replace so do nothing
+        return (TempStr)s;
+    }
+
+    size_t findLen = str::Len(toReplace);
+    size_t replLen = str::Len(replaceWith);
+    size_t lenDiff = 0;
+    if (replLen > findLen) {
+        lenDiff = replLen - findLen;
+    }
+    // heuristic: allow 6 replacements without reallocating
+    size_t capHint = str::Len(s) + 1 + (lenDiff * 6);
+    str::Str result(capHint);
     bool ok;
-    str::Str result(str::Len(s));
-    size_t findLen = str::Len(toReplace), replLen = str::Len(replaceWith);
-    const char *start = s, *end;
-    while ((end = str::Find(start, toReplace)) != nullptr) {
-        ok = result.Append(start, end - start);
+    while (end != nullptr) {
+        ok = result.Append(curr, end - curr);
         if (!ok) {
             return nullptr;
         }
         ok = result.Append(replaceWith, replLen);
         if (!ok) {
             return nullptr;
         }
-        start = end + findLen;
+        curr = end + findLen;
+        end = str::Find(curr, toReplace);
     }
-    ok = result.Append(start);
+    ok = result.Append(curr);
     if (!ok) {
         return nullptr;
     }
-    char* res = DupTemp(result.Get());
-    return res;
+    return result.StealData(GetTempAllocator());
 }
 
 } // namespace str

diff --git a/src/utils/WinUtil.cpp b/src/utils/WinUtil.cpp
@@ -1690,11 +1690,7 @@ void MenuSetText(HMENU m, int id, const char* s) {
    if no change is needed, the string is returned as is,
    else it's also saved in newResult for automatic freeing */
 TempStr MenuToSafeStringTemp(const char* s) {
-    auto str = str::DupTemp(s);
-    if (!str::FindChar(str, '&')) {
-        return str;
-    }
-    TempStr safe = str::ReplaceTemp(str, "&", "&&");
+    TempStr safe = str::ReplaceTemp(s, "&", "&&");
     return safe;
 }