Merge pull request #12048 from keymanapp/refactor/developer/kmcmplib-…

…yak-shave-more-bools refactor(developer): move error reporting inside `GetCompileTargetsFromTargetsStore` and `ProcessGroupFinish` 🌋
keymanapp · Aug 3, 2024 · 15829a8 · 15829a8
2 parents a01e0be + 59addc5
commit 15829a8
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 34 deletions.
diff --git a/developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp b/developer/src/kmcmplib/src/CompileKeyboardBuffer.cpp
@@ -136,7 +136,9 @@ bool CompileKeyboardBuffer(KMX_BYTE* infile, int sz, PFILE_KEYBOARD fk)
     return FALSE;
   }
 
-  ProcessGroupFinish(fk);
+  if(!ProcessGroupFinish(fk)) {
+    return FALSE;
+  }
 
   if (kmcmp::FSaveDebug) kmcmp::RecordDeadkeyNames(fk);
 

diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp
@@ -139,7 +139,7 @@ KMX_BOOL IsSameToken(PKMX_WCHAR *p, KMX_WCHAR const * token);
 KMX_DWORD GetRHS(PFILE_KEYBOARD fk, PKMX_WCHAR p, PKMX_WCHAR buf, int bufsize, int offset, int IsUnicode);
 PKMX_WCHAR GetDelimitedString(PKMX_WCHAR *p, KMX_WCHAR const * Delimiters, KMX_WORD Flags);
 KMX_DWORD GetXString(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_WCHAR const * token, PKMX_WCHAR output, int max, int offset, PKMX_WCHAR *newp, int isVKey, int isUnicode);
-KMX_DWORD GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets);
+KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets);
 
 int GetGroupNum(PFILE_KEYBOARD fk, PKMX_WCHAR p);
 
@@ -381,8 +381,7 @@ KMX_BOOL ParseLine(PFILE_KEYBOARD fk, PKMX_WCHAR str) {
     if (fk->currentGroup == 0xFFFFFFFF) {
       fk->currentGroup = 0;
     } else {
-      if ((msg = ProcessGroupFinish(fk)) != STATUS_Success) {
-        AddCompileError(msg);
+      if (!ProcessGroupFinish(fk)) {
         return FALSE;		// finish off previous group first?
       }
       fk->currentGroup++;
@@ -733,21 +732,27 @@ int kmcmp::cmpkeys(const void *key, const void *elem)
   return(char_key - char_elem); // akey->Key - aelem->Key);
 }
 
-KMX_DWORD ProcessGroupFinish(PFILE_KEYBOARD fk)
-{
+KMX_BOOL ProcessGroupFinish(PFILE_KEYBOARD fk) {
   PFILE_GROUP gp;
   KMX_DWORD msg;
 
-  if (fk->currentGroup == 0xFFFFFFFF) return STATUS_Success;
-  // Just got to first group - so nothing to finish yet
+  if (fk->currentGroup == 0xFFFFFFFF) {
+    // Just got to first group - so nothing to finish yet
+    return TRUE;
+  }
 
   gp = &fk->dpGroupArray[fk->currentGroup];
 
   // Finish off the previous group stuff!
-  if ((msg = ExpandCapsRulesForGroup(fk, gp)) != STATUS_Success) return msg;
+  if ((msg = ExpandCapsRulesForGroup(fk, gp)) != STATUS_Success) {
+    AddCompileError(msg);
+    return FALSE;
+  }
   qsort(gp->dpKeyArray, gp->cxKeyArray, sizeof(FILE_KEY), kmcmp::cmpkeys);
 
-  return VerifyUnreachableRules(gp);
+  VerifyUnreachableRules(gp);
+
+  return TRUE;
 }
 
 /***************************************
@@ -1179,8 +1184,7 @@ KMX_BOOL ProcessSystemStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, PFILE_STORE s
 
   case TSS_TARGETS:   // I4504
     VERIFY_KEYBOARD_VERSION(fk, VERSION_90, KmnCompilerMessages::ERROR_90FeatureOnlyTargets);
-    if((msg = GetCompileTargetsFromTargetsStore(sp->dpString, fk->extra->targets)) != STATUS_Success) {
-      AddCompileError(msg);
+    if(!GetCompileTargetsFromTargetsStore(sp->dpString, fk->extra->targets)) {
       return FALSE;
     }
     break;
@@ -1269,7 +1273,7 @@ KMX_BOOL ProcessSystemStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, PFILE_STORE s
   return TRUE;
 }
 
-KMX_DWORD GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets) {
+KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets) {
   // Compile to .kmx
   const std::vector<std::u16string> KMXKeymanTargets{
     u"windows", u"macosx", u"linux", u"desktop"
@@ -1312,18 +1316,20 @@ KMX_DWORD GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets
         snprintf(ErrExtraLIB, ERR_EXTRA_LIB_LEN, " target: %s", string_from_u16string(token).c_str());
         delete[] p;
         targets = 0;
-        return KmnCompilerMessages::ERROR_InvalidTarget;
+        AddCompileError(KmnCompilerMessages::ERROR_InvalidTarget);
+        return FALSE;
       }
     }
     token = u16tok(nullptr, u" ", &ctx);
   }
   delete[] p;
 
   if(targets == 0) {
-    return KmnCompilerMessages::ERROR_NoTargetsSpecified;
+    AddCompileError(KmnCompilerMessages::ERROR_NoTargetsSpecified);
+    return FALSE;
   }
 
-  return STATUS_Success;
+  return TRUE;
 }
 
 KMX_BOOL IsValidKeyboardVersion(KMX_WCHAR *dpString) {   // I4140

diff --git a/developer/src/kmcmplib/src/UnreachableRules.cpp b/developer/src/kmcmplib/src/UnreachableRules.cpp
@@ -25,7 +25,8 @@ namespace kmcmp {
   }
 
 }
-KMX_DWORD VerifyUnreachableRules(PFILE_GROUP gp) {
+
+void VerifyUnreachableRules(PFILE_GROUP gp) {
   PFILE_KEY kp = gp->dpKeyArray;
   KMX_DWORD i;
 
@@ -51,6 +52,4 @@ KMX_DWORD VerifyUnreachableRules(PFILE_GROUP gp) {
   }
 
   kmcmp::currentLine = oldCurrentLine;
-
-  return STATUS_Success;
 }
diff --git a/developer/src/kmcmplib/src/UnreachableRules.h b/developer/src/kmcmplib/src/UnreachableRules.h
@@ -1,3 +1,3 @@
 #pragma once
 
-KMX_DWORD VerifyUnreachableRules(PFILE_GROUP gp);
+void VerifyUnreachableRules(PFILE_GROUP gp);
diff --git a/developer/src/kmcmplib/src/kmcmplib.h b/developer/src/kmcmplib/src/kmcmplib.h
@@ -34,7 +34,7 @@ KMX_BOOL AddStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, const KMX_WCHAR * str,
 KMX_DWORD ReadLine(KMX_BYTE* infile, int sz, int& offset, PKMX_WCHAR wstr, KMX_BOOL PreProcess);
 KMX_BOOL ParseLine(PFILE_KEYBOARD fk, PKMX_WCHAR str);
 KMX_BOOL ProcessGroupLine(PFILE_KEYBOARD fk, PKMX_WCHAR p);
-KMX_DWORD ProcessGroupFinish(PFILE_KEYBOARD fk);
+KMX_BOOL ProcessGroupFinish(PFILE_KEYBOARD fk);
 KMX_BOOL ProcessStoreLine(PFILE_KEYBOARD fk, PKMX_WCHAR p);
 int LineTokenType(PKMX_WCHAR *str);
 KMX_BOOL BuildVKDictionary(PFILE_KEYBOARD fk); // I3438
diff --git a/developer/src/kmcmplib/tests/api-test.cpp b/developer/src/kmcmplib/tests/api-test.cpp
@@ -34,6 +34,7 @@ int main(int argc, char *argv[]) {
   setup();
   test_kmcmp_CompileKeyboard(argv[1]);
 
+  setup();
   test_GetCompileTargetsFromTargetsStore();
 
   return 0;
@@ -70,30 +71,67 @@ void test_kmcmp_CompileKeyboard(char *kmn_file) {
   unlink(kmn_file);
 }
 
-extern KMX_DWORD GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets);
+extern KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets);
 
 void test_GetCompileTargetsFromTargetsStore() {
   int targets = 0;
-  assert(GetCompileTargetsFromTargetsStore(u"any", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"any", targets));
+  assert(error_vec.size() == 0);
   assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
-  assert(GetCompileTargetsFromTargetsStore(u"windows", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"windows", targets));
+  assert(error_vec.size() == 0);
   assert(targets == COMPILETARGETS_KMX);
-  assert(GetCompileTargetsFromTargetsStore(u"desktop", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"desktop", targets));
+  assert(error_vec.size() == 0);
   assert(targets == COMPILETARGETS_KMX);
-  assert(GetCompileTargetsFromTargetsStore(u"mobile", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"mobile", targets));
+  assert(error_vec.size() == 0);
   assert(targets == COMPILETARGETS_JS);
-  assert(GetCompileTargetsFromTargetsStore(u"web", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"web", targets));
+  assert(error_vec.size() == 0);
   assert(targets == COMPILETARGETS_JS);
-  assert(GetCompileTargetsFromTargetsStore(u"desktop mobile", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"desktop mobile", targets));
+  assert(error_vec.size() == 0);
   assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
-  assert(GetCompileTargetsFromTargetsStore(u"desktop   tablet", targets) == STATUS_Success);
+
+  setup();
+  assert(GetCompileTargetsFromTargetsStore(u"desktop   tablet", targets));
+  assert(error_vec.size() == 0);
   assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
-  assert(GetCompileTargetsFromTargetsStore(u"foo bar baz", targets) == KmnCompilerMessages::ERROR_InvalidTarget);
+
+  setup();
+  assert(!GetCompileTargetsFromTargetsStore(u"foo bar baz", targets));
+  assert(error_vec.size() == 1);
+  assert(error_vec[0] == KmnCompilerMessages::ERROR_InvalidTarget);
   assert(targets == 0);
-  assert(GetCompileTargetsFromTargetsStore(u"windows chromeos", targets) == KmnCompilerMessages::ERROR_InvalidTarget);
+
+  setup();
+  assert(!GetCompileTargetsFromTargetsStore(u"windows chromeos", targets));
+  assert(error_vec.size() == 1);
+  assert(error_vec[0] == KmnCompilerMessages::ERROR_InvalidTarget);
   assert(targets == 0);
-  assert(GetCompileTargetsFromTargetsStore(u" ", targets) == KmnCompilerMessages::ERROR_NoTargetsSpecified);
+
+  setup();
+  assert(!GetCompileTargetsFromTargetsStore(u" ", targets));
+  assert(error_vec.size() == 1);
+  assert(error_vec[0] == KmnCompilerMessages::ERROR_NoTargetsSpecified);
   assert(targets == 0);
-  assert(GetCompileTargetsFromTargetsStore(u"", targets) == KmnCompilerMessages::ERROR_NoTargetsSpecified);
+
+  setup();
+  assert(!GetCompileTargetsFromTargetsStore(u"", targets));
+  assert(error_vec.size() == 1);
+  assert(error_vec[0] == KmnCompilerMessages::ERROR_NoTargetsSpecified);
   assert(targets == 0);
 }
diff --git a/developer/src/kmcmplib/tests/gtest-compiler-test.cpp b/developer/src/kmcmplib/tests/gtest-compiler-test.cpp
@@ -282,7 +282,7 @@ TEST_F(CompilerTest, ValidateMatchNomatchOutput_test) {
 // KMX_BOOL ParseLine(PFILE_KEYBOARD fk, PKMX_WCHAR str)
 // KMX_BOOL ProcessGroupLine(PFILE_KEYBOARD fk, PKMX_WCHAR p)
 // int kmcmp::cmpkeys(const void *key, const void *elem)
-// KMX_DWORD ProcessGroupFinish(PFILE_KEYBOARD fk)
+// KMX_BOOL ProcessGroupFinish(PFILE_KEYBOARD fk)
 // KMX_BOOL ProcessStoreLine(PFILE_KEYBOARD fk, PKMX_WCHAR p)
 // bool resizeStoreArray(PFILE_KEYBOARD fk)
 // bool resizeKeyArray(PFILE_GROUP gp, int increment)