fix(developer): handle leading delimiters correctly in u16tok()

common/cpp/km_u16.cpp

@@ -326,10 +326,20 @@ int u16ncmp(const KMX_WCHAR* p, const KMX_WCHAR* q, size_t count) {
  * @return Pointer to the first token in p
  */
 KMX_WCHAR* u16tok(KMX_WCHAR* p, const KMX_WCHAR ch, KMX_WCHAR** ctx) {
+  if(ch == 0 || !ctx) {
+    return NULL;
+  }
+
   if (!p) {
     p = *ctx;
-    if (!p)
+    if (!p) {
       return NULL;
+    }
+  }
+
+  // skip initial delimiter
+  while (*p == ch) {
+    p++;
   }
 
   KMX_WCHAR* q = p;
@@ -356,10 +366,20 @@ KMX_WCHAR* u16tok(KMX_WCHAR* p, const KMX_WCHAR ch, KMX_WCHAR** ctx) {
  * @return Pointer to the first token in p
  */
 KMX_WCHAR* u16tok(KMX_WCHAR* p, const KMX_WCHAR* delimiters, KMX_WCHAR** ctx) {
+  if(!ctx || !delimiters || !(*delimiters)) {
+    return NULL;
+  }
+
   if (!p) {
     p = *ctx;
-    if (!p)
+    if (!p) {
       return NULL;
+    }
+  }
+
+  // skip initial delimiters
+  while (*p && u16chr(delimiters, *p)) {
+    p++;
   }
 
   KMX_WCHAR* q = p;

developer/src/kmc-kmn/test/compiler.tests.ts

@@ -1,10 +1,15 @@
+/*
+ * Keyman is copyright (C) SIL Global. MIT License.
+ */
+import { dirname } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import fs from 'node:fs';
 import 'mocha';
 import { assert } from 'chai';
-import { KmnCompiler } from '../src/main.js';
-import { dirname } from 'path';
-import { fileURLToPath } from 'url';
-import fs from 'fs';
 import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers';
+import { KmxFileReader } from '@keymanapp/common-types';
+import { compileTestKeyboard } from './helpers/index.js';
+import { KmnCompiler } from '../src/main.js';
 
 const __dirname = dirname(fileURLToPath(import.meta.url)).replace(/\\/g, '/');
 const keyboardsDir = __dirname + '/../../../../../common/test/keyboards/';
@@ -13,6 +18,16 @@ const baselineDir = keyboardsDir + 'baseline/';
 describe('Compiler class', function() {
   const callbacks = new TestCompilerCallbacks(this);
 
+  this.beforeEach(function() {
+    callbacks.clear();
+  });
+
+  this.afterEach(function() {
+    if(this.currentTest?.isFailed()) {
+      callbacks.printMessages();
+    }
+  });
+
   it('should throw on failure', async function() {
     const compiler = new KmnCompiler();
     const callbacks : any = null; // ERROR
@@ -129,4 +144,20 @@ describe('Compiler class', function() {
     assert.deepEqual(kvkData, kvkFixtureData);
   });
 
+  it('should trim all whitespace for `&targets` store', async function() {
+    const result = await compileTestKeyboard(callbacks, ['keyboards', 'targets-with-whitespace.kmn']);
+    assert.isNotNull(result);
+
+    // Verify implictly that `&targets` store was interpreted correctly as 'any'
+    // because the compiler generated both JS and KMX targets (#13721)
+    assert.isNotNull(result.artifacts.js);
+    assert.isNotNull(result.artifacts.kmx);
+
+    // Then verify directly that the `&targets` store was trimmed by looking at
+    // the final value in the kmx data
+    const reader = new KmxFileReader();
+    const keyboard = reader.read(result.artifacts.kmx.data);
+    assert.equal(keyboard.targets, 'any');
+  });
+
 });

developer/src/kmc-kmn/test/fixtures/keyboards/targets-with-whitespace.kmn

@@ -0,0 +1,11 @@
+store(&NAME) 'targets_with_whitespace'
+store(&VERSION) '10.0'
+
+c this used to generate KM0207B: At least one compile target must be specified
+store(&TARGETS) ' any'
+
+begin unicode > use(main)
+
+group(main) using keys
+
++ 'x' > 'y'

developer/src/kmc-kmn/test/helpers/index.ts

@@ -1,8 +1,13 @@
-/**
+/*
+ * Keyman is copyright (C) SIL Global. MIT License.
+ *
  * Helpers and utilities for the Mocha tests.
  */
-import * as path from 'path';
-import { fileURLToPath } from 'url';
+import * as path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { assert } from 'chai';
+import { CompilerCallbacks } from '@keymanapp/developer-utils';
+import { KmnCompiler } from '../../src/compiler/compiler.js';
 
 /**
  * Builds a path to the fixture with the given path components.
@@ -15,3 +20,13 @@ import { fileURLToPath } from 'url';
  export function makePathToFixture(...components: string[]): string {
   return fileURLToPath(new URL(path.join('..', '..', '..', 'test', 'fixtures', ...components), import.meta.url));
 }
+
+export async function compileTestKeyboard(callbacks: CompilerCallbacks, fixture: string[]) {
+  const compiler = new KmnCompiler();
+  assert(await compiler.init(callbacks, {saveDebug: true, shouldAddCompilerVersion: false}));
+  assert(compiler.verifyInitialized());
+
+  const kmnPath = makePathToFixture(...fixture);
+
+  return await compiler.run(kmnPath, null);
+}

developer/src/kmcmplib/src/Compiler.cpp

@@ -138,7 +138,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_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets);
+KMX_BOOL GetCompileTargetsFromTargetsStore(KMX_WCHAR *store, int &targets);
 
 int GetGroupNum(PFILE_KEYBOARD fk, PKMX_WCHAR p);
 
@@ -1331,7 +1331,16 @@ KMX_BOOL ProcessSystemStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, PFILE_STORE s
   return TRUE;
 }
 
-KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets) {
+/**
+ * Extract the compile targets from the &targets store, and rewrite the
+ * &targets dpString value to remove unnecessary whitespace. Does not
+ * reallocate sp->dpString, but overwrites its value with a string the
+ * same length or shorter.
+ * @param store       store value to rewrite
+ * @param targets     (output)
+ * @return FALSE if no targets found or invalid targets found
+ */
+KMX_BOOL GetCompileTargetsFromTargetsStore(KMX_WCHAR *store, int &targets) {
   // Compile to .kmx
   const std::vector<std::u16string> KMXKeymanTargets{
     u"windows", u"macosx", u"linux", u"desktop"
@@ -1347,10 +1356,17 @@ KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets)
 
   targets = 0;
   auto p = new KMX_WCHAR[u16len(store)+1];
-  u16cpy(p, store);
+  auto q = p;
+  *p = 0;
   KMX_WCHAR* ctx;
-  auto token = u16tok(p, u" ", &ctx);
+  auto token = u16tok(store, u" ", &ctx);
   while(token) {
+    if(q > p) {
+      // Insert a delimiter between tokens, if
+      // more than one token
+      *q++ = ' ';
+      *q = 0;
+    }
     bool found = false;
     if(*token) {
       if(AnyTarget == token) {
@@ -1378,9 +1394,16 @@ KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets)
         targets = 0;
         return FALSE;
       }
+
+      // Append the token to the output, we know it is long enough
+      // because the buffer is the same length as the input
+      u16cpy(q, token);
+      q = const_cast<KMX_WCHAR*>(u16chr(q, 0));
+      *q = 0;
     }
     token = u16tok(nullptr, u" ", &ctx);
   }
+  u16cpy(store, p);
   delete[] p;
 
   if(targets == 0) {

developer/src/kmcmplib/tests/api.tests.cpp

@@ -71,66 +71,90 @@ void test_kmcmp_CompileKeyboard(char *kmn_file) {
   unlink(kmn_file);
 }
 
-extern KMX_BOOL GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets);
+extern KMX_BOOL GetCompileTargetsFromTargetsStore(KMX_WCHAR* buf, int &targets);
+
+KMX_BOOL do_GetCompileTargetsFromTargetsStore(const KMX_WCHAR* store, int &targets, const KMX_WCHAR* expected = nullptr) {
+  KMX_WCHAR* buf;
+  buf = new KMX_WCHAR[u16len(store)+1];
+  u16cpy(buf, store);
+  KMX_BOOL result = GetCompileTargetsFromTargetsStore(buf, targets);
+
+  if(expected) {
+    test_assert(u16cmp(expected, buf) == 0);
+  }
+
+  delete[] buf;
+  return result;
+}
 
 void test_GetCompileTargetsFromTargetsStore() {
   int targets = 0;
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"any", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"any", targets, u"any"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"windows", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"windows", targets, u"windows"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == COMPILETARGETS_KMX);
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"desktop", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"desktop", targets, u"desktop"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == COMPILETARGETS_KMX);
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"mobile", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"mobile", targets, u"mobile"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == COMPILETARGETS_JS);
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"web", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"web", targets, u"web"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == COMPILETARGETS_JS);
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"desktop mobile", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"desktop mobile", targets, u"desktop mobile"));
+  test_assert(error_vec.size() == 0);
+  test_assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
+
+  setup();
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"desktop   tablet", targets, u"desktop tablet"));
+  test_assert(error_vec.size() == 0);
+  test_assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
+
+  setup();
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"  desktop   tablet", targets, u"desktop tablet"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
 
   setup();
-  test_assert(GetCompileTargetsFromTargetsStore(u"desktop   tablet", targets));
+  test_assert(do_GetCompileTargetsFromTargetsStore(u"  windows      androidphone  ", targets, u"windows androidphone"));
   test_assert(error_vec.size() == 0);
   test_assert(targets == (COMPILETARGETS_KMX | COMPILETARGETS_JS));
 
   setup();
-  test_assert(!GetCompileTargetsFromTargetsStore(u"foo bar baz", targets));
+  test_assert(!do_GetCompileTargetsFromTargetsStore(u"foo bar baz", targets));
   test_assert(error_vec.size() == 1);
   test_assert(error_vec[0] == KmnCompilerMessages::ERROR_InvalidTarget);
   test_assert(targets == 0);
 
   setup();
-  test_assert(!GetCompileTargetsFromTargetsStore(u"windows chromeos", targets));
+  test_assert(!do_GetCompileTargetsFromTargetsStore(u"windows chromeos", targets));
   test_assert(error_vec.size() == 1);
   test_assert(error_vec[0] == KmnCompilerMessages::ERROR_InvalidTarget);
   test_assert(targets == 0);
 
   setup();
-  test_assert(!GetCompileTargetsFromTargetsStore(u" ", targets));
+  test_assert(!do_GetCompileTargetsFromTargetsStore(u" ", targets));
   test_assert(error_vec.size() == 1);
   test_assert(error_vec[0] == KmnCompilerMessages::ERROR_NoTargetsSpecified);
   test_assert(targets == 0);
 
   setup();
-  test_assert(!GetCompileTargetsFromTargetsStore(u"", targets));
+  test_assert(!do_GetCompileTargetsFromTargetsStore(u"", targets));
   test_assert(error_vec.size() == 1);
   test_assert(error_vec[0] == KmnCompilerMessages::ERROR_NoTargetsSpecified);
   test_assert(targets == 0);

developer/src/kmcmplib/tests/gtest-km_u16.tests.cpp

@@ -32,6 +32,11 @@ TEST(km_u16_Test, u16tok_char_delim) {
 	KMX_WCHAR str[LINESIZE];
   KMX_WCHAR *ctx = nullptr;
 
+  // invalid parameters
+  EXPECT_EQ(nullptr, u16tok(str, ' ', nullptr));
+  EXPECT_EQ(nullptr, u16tok(str, (KMX_WCHAR)0, &ctx));
+  EXPECT_EQ(nullptr, u16tok(nullptr, ' ', nullptr));
+
   // sequence of tokens
   u16cpy(str, u"test a space  and two");
   ctx = nullptr;
@@ -55,6 +60,13 @@ TEST(km_u16_Test, u16tok_char_delim) {
   EXPECT_TRUE(!u16cmp(u"b", u16tok(nullptr, ' ', &ctx)));
   EXPECT_EQ(nullptr, u16tok(nullptr, ' ', &ctx));
 
+  // delimiters at start
+  u16cpy(str, u"  a b");
+  ctx = nullptr;
+  EXPECT_TRUE(!u16cmp(u"a", u16tok(str, ' ', &ctx)));
+  EXPECT_TRUE(!u16cmp(u"b", u16tok(nullptr, ' ', &ctx)));
+  EXPECT_EQ(nullptr, u16tok(nullptr, ' ', &ctx));
+
   // no string, no context
   ctx = nullptr;
   EXPECT_EQ(nullptr, u16tok(nullptr, ' ', &ctx));
@@ -73,6 +85,12 @@ TEST(km_u16_Test, u16tok_str_delim) {
 	KMX_WCHAR str[LINESIZE];
   KMX_WCHAR *ctx = nullptr;
 
+  // invalid parameters
+  EXPECT_EQ(nullptr, u16tok(str, u" ", nullptr));
+  EXPECT_EQ(nullptr, u16tok(str, nullptr, &ctx));
+  EXPECT_EQ(nullptr, u16tok(str, u"", &ctx));
+  EXPECT_EQ(nullptr, u16tok(nullptr, u" ", nullptr));
+
   // sequence of tokens
   u16cpy(str, u"test a space  and two");
   ctx = nullptr;
@@ -96,6 +114,13 @@ TEST(km_u16_Test, u16tok_str_delim) {
   EXPECT_TRUE(!u16cmp(u"b", u16tok(nullptr, u" ", &ctx)));
   EXPECT_EQ(nullptr, u16tok(nullptr, u" ", &ctx));
 
+  // delimiters at start
+  u16cpy(str, u"  a b");
+  ctx = nullptr;
+  EXPECT_TRUE(!u16cmp(u"a", u16tok(str, u" ", &ctx)));
+  EXPECT_TRUE(!u16cmp(u"b", u16tok(nullptr, u" ", &ctx)));
+  EXPECT_EQ(nullptr, u16tok(nullptr, ' ', &ctx));
+
   // no string, no context
   ctx = nullptr;
 	EXPECT_EQ(nullptr, u16tok(nullptr, u"", &ctx));
@@ -130,6 +155,9 @@ TEST(km_u16_Test, u16tok_str_compare_to_strtok) {
   // Compare behaviour of strtok:
 	char str[LINESIZE];
 
+  // Note: strtok behavior is undefined with invalid parameters so we don't do a reference test
+  // of invalid parameters
+
   // sequence of tokens
   strcpy(str, "test a space  and two");
   EXPECT_TRUE(!strcmp("test", strtok(str, " ")));
@@ -150,6 +178,12 @@ TEST(km_u16_Test, u16tok_str_compare_to_strtok) {
   EXPECT_TRUE(!strcmp("b", strtok(nullptr, " ")));
   EXPECT_EQ(nullptr, strtok(nullptr, " "));
 
+  // delimiters at start
+  strcpy(str, "   a b");
+  EXPECT_TRUE(!strcmp("a", strtok(str, " ")));
+  EXPECT_TRUE(!strcmp("b", strtok(nullptr, " ")));
+  EXPECT_EQ(nullptr, strtok(nullptr, " "));
+
   // multiple delimiters
   strcpy(str, "abc<def>ghi");
   const char *delim = "<>";