Skip to content

Commit

Permalink
fix(developer): kmc code generation for context(n) in context
Browse files Browse the repository at this point in the history
Fixes #9930.

* Fixes the index offset calculation, with +1 for >=v10.0 keyboards and
  -1 for <v10.0 keyboards (difference is because older version keyboards
  have context offsets calculated in the opposite direction). Note that
  this happened because the `CODE_CONTEXTEX` index is stored in memory
  as 0-based in kmc, but was 1-based in kmcomp.

* Adds unit tests for v9.0 and v10.0 keyboards for `context(n)` in
  context.

* Adds unit tests for v9.0 and v10.0 keyboards for `context(n)` in
  output. These were already correct but now we can be assured that
  they are being tested.

* Enables code coverage for kmw-compiler, and sets a lower threshold for
  pass because coverage is still just over 70%.
  • Loading branch information
mcdurdin committed Nov 3, 2023
1 parent cd5be3d commit 186132d
Show file tree
Hide file tree
Showing 12 changed files with 270 additions and 8 deletions.
6 changes: 5 additions & 1 deletion developer/src/kmc-kmn/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,11 @@ if builder_start_action test; then
copy_deps
tsc --build test/
npm run lint
c8 --reporter=lcov --reporter=text --exclude-after-remap mocha "${builder_extra_params[@]}"
readonly C8_THRESHOLD=70
c8 --reporter=lcov --reporter=text --lines $C8_THRESHOLD --statements $C8_THRESHOLD --branches $C8_THRESHOLD --functions $C8_THRESHOLD mocha
builder_echo warning "Coverage thresholds are currently $C8_THRESHOLD%, which is lower than ideal."
builder_echo warning "Please increase threshold in build.sh as test coverage improves."

builder_finish_action success test
fi

Expand Down
1 change: 0 additions & 1 deletion developer/src/kmc-kmn/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
"exclude-after-remap": true,
"exclude": [
"src/import/",
"src/kmw-compiler",
"test/"
]
},
Expand Down
4 changes: 2 additions & 2 deletions developer/src/kmc-kmn/src/kmw-compiler/javascript-strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ function JavaScript_CompositeContextValue(fk: KMX.KEYBOARD, fkp: KMX.KEY, pwsz:
Cur--; // don't increment on ifsystemstore -- correlates with AdjustIndex function // I3910
break;
case KMX.KMXFile.CODE_CONTEXTEX: // I3980
Result += `k.KCCM(${Len-Cur},${Len-rec.ContextEx.Index+1},t)`;
Result += `k.KCCM(${Len-Cur},${Len-rec.ContextEx.Index},t)`;
break;
case KMX.KMXFile.CODE_NOTANY: // I3981
CheckStoreForInvalidFunctions(fk, fkp, rec.Any.Store); // I1520
Expand Down Expand Up @@ -572,7 +572,7 @@ function JavaScript_FullContextValue(fk: KMX.KEYBOARD, fkp: KMX.KEY, pwsz: strin
FullContext += `{t:'a',a:this.s${JavaScript_Name(rec.Any.StoreIndex, rec.Any.Store.dpName)},n:1}`;
break;
case KMX.KMXFile.CODE_CONTEXTEX:
FullContext += `{t:'c',c:${rec.ContextEx.Index}}`; // I4611
FullContext += `{t:'c',c:${rec.ContextEx.Index+1}}`; // I4611
break;
case KMX.KMXFile.CODE_INDEX:
FullContext += `{t:'i',i:this.s${JavaScript_Name(rec.Index.StoreIndex, rec.Index.Store.dpName)},`+
Expand Down
54 changes: 54 additions & 0 deletions developer/src/kmc-kmn/test/fixtures/kmw/test_context_in_context.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
if(typeof keyman === 'undefined') {
console.log('Keyboard requires KeymanWeb 10.0 or later');
if(typeof tavultesoft !== 'undefined') tavultesoft.keymanweb.util.alert("This keyboard requires KeymanWeb 10.0 or later");
} else {
KeymanWeb.KR(new Keyboard_test_context_in_context());
}
function Keyboard_test_context_in_context()
{
var modCodes = keyman.osk.modifierCodes;
var keyCodes = keyman.osk.keyCodes;

this._v=(typeof keyman!="undefined"&&typeof keyman.version=="string")?parseInt(keyman.version,10):9;
this.KI="Keyboard_test_context_in_context";
this.KN="test context(n) in context, #9930";
this.KMINVER="10.0";
this.KV=null;
this.KDU=0;
this.KH='';
this.KM=0;
this.KBVER="1.0";
this.KMBM=modCodes.SHIFT /* 0x0010 */;
this.s_liveQwerty_6="qwerty";
this.s_deadQwerty_7=[{t:'d',d:0},{t:'d',d:1},{t:'d',d:2},{t:'d',d:3},{t:'d',d:4},{t:'d',d:5}];
this.KVS=[];
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.g_main_0=function(t,e) {
var k=KeymanWeb,r=0,m=0;
if(k.KKM(e, modCodes.SHIFT | modCodes.VIRTUAL_KEY /* 0x4010 */, keyCodes.K_1 /* 0x31 */)) {
if(k.KFCM(1,t,[{t:'a',a:this.s_liveQwerty_6}])){
r=m=1; // Line 13
k.KDC(1,t);
k.KO(-1,t,"?");
k.KIO(-1,this.s_deadQwerty_7,1,t);
k.KIO(-1,this.s_deadQwerty_7,1,t);
}
}
else if(k.KKM(e, modCodes.VIRTUAL_KEY /* 0x4000 */, keyCodes.K_PERIOD /* 0xBE */)) {
if(k.KFCM(3,t,['?',{t:'a',a:this.s_deadQwerty_7},{t:'c',c:2}])){
r=m=1; // Line 17
k.KDC(3,t);
k.KO(-1,t,"(");
k.KIO(-1,this.s_liveQwerty_6,2,t);
k.KIO(-1,this.s_liveQwerty_6,2,t);
k.KO(-1,t,")");
}
}
return r;
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
store(&VERSION) "10.0"
store(&TARGETS) 'web'
store(&NAME) 'test context(n) in context, #9930'

begin Unicode > use(main)

group(main) using keys

store(liveQwerty) 'qwerty'
store(deadQwerty) dk(q) dk(w) dk(e) dk(r) dk(t) dk(y)

c any/index with deadkeys in stores.
any(liveQwerty) + '!' > '?' index(deadQwerty, 1) index(deadQwerty, 1)

c The rule below is misgenerated by kmc-17.0.205-alpha on `context(2)` statement;
c see https://github.com/keymanapp/keyman/issues/9930
'?' any(deadQwerty) context(2) + '.' > '(' index(liveQwerty, 2) index(liveQwerty, 2) ')'
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@

KeymanWeb.KR(new Keyboard_test_context_in_context_9());

function Keyboard_test_context_in_context_9()
{

this._v=(typeof keyman!="undefined"&&typeof keyman.version=="string")?parseInt(keyman.version,10):9;
this.KI="Keyboard_test_context_in_context_9";
this.KN="test context(n) in context, #9930, v9.0";
this.KMINVER="9.0";
this.KV=null;
this.KDU=0;
this.KH='';
this.KM=0;
this.KBVER="1.0";
this.KMBM=0x0010;
this.s_liveQwerty_6="qwerty";
this.s_deadQwerty_7="123456";
this.KVS=[];
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.g_main_0=function(t,e) {
var k=KeymanWeb,r=0,m=0;
if(k.KKM(e, 0x4010, 0x31)) {
if(k.KA(0,k.KC(1,1,t),this.s_liveQwerty_6)){
r=m=1; // Line 13
k.KO(1,t,"?");
k.KIO(-1,this.s_deadQwerty_7,1,t);
k.KIO(-1,this.s_deadQwerty_7,1,t);
}
}
else if(k.KKM(e, 0x4000, 0xBE)) {
if(k.KCM(3,t,"?",1)&&k.KA(1,k.KC(2,1,t),this.s_deadQwerty_7)&&k.KCCM(1,2,t)){
r=m=1; // Line 17
k.KO(3,t,"(");
k.KIO(-1,this.s_liveQwerty_6,2,t);
k.KIO(-1,this.s_liveQwerty_6,2,t);
k.KO(-1,t,")");
}
}
return r;
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
store(&VERSION) "9.0"
store(&TARGETS) 'web'
store(&NAME) 'test context(n) in context, #9930, v9.0'

begin Unicode > use(main)

group(main) using keys

store(liveQwerty) 'qwerty'
store(deadQwerty) '123456'

c any/index with deadkeys in stores.
any(liveQwerty) + '!' > '?' index(deadQwerty, 1) index(deadQwerty, 1)

c The rule below is misgenerated by kmc-17.0.205-alpha on `context(2)` statement;
c see https://github.com/keymanapp/keyman/issues/9930
'?' any(deadQwerty) context(2) + '.' > '(' index(liveQwerty, 2) index(liveQwerty, 2) ')'
44 changes: 44 additions & 0 deletions developer/src/kmc-kmn/test/fixtures/kmw/test_contextn_in_output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
if(typeof keyman === 'undefined') {
console.log('Keyboard requires KeymanWeb 10.0 or later');
if(typeof tavultesoft !== 'undefined') tavultesoft.keymanweb.util.alert("This keyboard requires KeymanWeb 10.0 or later");
} else {
KeymanWeb.KR(new Keyboard_test_contextn_in_output());
}
function Keyboard_test_contextn_in_output()
{
var modCodes = keyman.osk.modifierCodes;
var keyCodes = keyman.osk.keyCodes;

this._v=(typeof keyman!="undefined"&&typeof keyman.version=="string")?parseInt(keyman.version,10):9;
this.KI="Keyboard_test_contextn_in_output";
this.KN="test context(n) in output, #9930, v10.0";
this.KMINVER="10.0";
this.KV=null;
this.KDU=0;
this.KH='';
this.KM=0;
this.KBVER="1.0";
this.KMBM=modCodes.SHIFT /* 0x0010 */;
this.s_liveQwerty_6="qwerty";
this.s_deadQwerty_7=[{t:'d',d:0},{t:'d',d:1},{t:'d',d:2},{t:'d',d:3},{t:'d',d:4},{t:'d',d:5}];
this.KVS=[];
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.g_main_0=function(t,e) {
var k=KeymanWeb,r=0,m=0;
if(k.KKM(e, modCodes.SHIFT | modCodes.VIRTUAL_KEY /* 0x4010 */, keyCodes.K_1 /* 0x31 */)) {
if(k.KFCM(2,t,[{t:'a',a:this.s_liveQwerty_6},'1'])){
r=m=1; // Line 13
k.KDC(2,t);
k.KO(-1,t,"?");
k.KIO(-1,this.s_deadQwerty_7,1,t);
k.KO(-1,t,"1");
}
}
return r;
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
store(&VERSION) "10.0"
store(&TARGETS) 'web'
store(&NAME) 'test context(n) in output, #9930, v10.0'

begin Unicode > use(main)

group(main) using keys

store(liveQwerty) 'qwerty'
store(deadQwerty) dk(q) dk(w) dk(e) dk(r) dk(t) dk(y)

c any/index with deadkeys in stores.
any(liveQwerty) '1' + '!' > '?' index(deadQwerty, 1) context(2)
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@

KeymanWeb.KR(new Keyboard_test_contextn_in_output_9());

function Keyboard_test_contextn_in_output_9()
{

this._v=(typeof keyman!="undefined"&&typeof keyman.version=="string")?parseInt(keyman.version,10):9;
this.KI="Keyboard_test_contextn_in_output_9";
this.KN="test context(n) in output, #9930, v9.0";
this.KMINVER="9.0";
this.KV=null;
this.KDU=0;
this.KH='';
this.KM=0;
this.KBVER="1.0";
this.KMBM=0x0010;
this.s_liveQwerty_6="qwerty";
this.s_deadQwerty_7="123456";
this.KVS=[];
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.gs=function(t,e) {
return this.g_main_0(t,e);
};
this.g_main_0=function(t,e) {
var k=KeymanWeb,r=0,m=0;
if(k.KKM(e, 0x4010, 0x31)) {
if(k.KA(0,k.KC(2,1,t),this.s_liveQwerty_6)&&k.KCM(1,t,"1",1)){
r=m=1; // Line 13
k.KO(2,t,"?");
k.KIO(-1,this.s_deadQwerty_7,1,t);
k.KO(-1,t,"1");
}
}
return r;
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
store(&VERSION) "9.0"
store(&TARGETS) 'web'
store(&NAME) 'test context(n) in output, #9930, v9.0'

begin Unicode > use(main)

group(main) using keys

store(liveQwerty) 'qwerty'
store(deadQwerty) '123456'

c any/index with deadkeys in stores.
any(liveQwerty) '1' + '!' > '?' index(deadQwerty, 1) context(2)
24 changes: 20 additions & 4 deletions developer/src/kmc-kmn/test/kmw/test-kmw-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ describe('KeymanWeb Compiler', function() {
callbacks.clear();
});

it('should compile a complex keyboard', async function() {
it('should compile a complex keyboard', function() {
run_test_keyboard(kmnCompiler, 'khmer_angkor');
});

it('should handle option stores', async function() {
it('should handle option stores', function() {
//
// This is enough to verify that the option store is set appropriately with
// KLOAD because the fixture has that code present:
Expand All @@ -49,17 +49,33 @@ describe('KeymanWeb Compiler', function() {
run_test_keyboard(kmnCompiler, 'test_options');
});

it('should translate every "character style" key correctly', async function() {
it('should translate every "character style" key correctly', function() {
//
// This is enough to verify that every character style key is encoded in the
// same way as the fixture.
//
run_test_keyboard(kmnCompiler, 'test_keychars');
});

it('should handle readonly groups', async function() {
it('should handle readonly groups', function() {
run_test_keyboard(kmnCompiler, 'test_readonly_groups');
});

it('should handle context(n) in output of rule, v10.0 generation', function() {
run_test_keyboard(kmnCompiler, 'test_contextn_in_output');
});

it('should handle context(n) in output of rule, v9.0 generation', function() {
run_test_keyboard(kmnCompiler, 'test_contextn_in_output_9');
});

it('should handle context(n) in context part of rule, v9.0 generation', function() {
run_test_keyboard(kmnCompiler, 'test_context_in_context_9');
});

it('should handle context(n) in context part of rule, v10.0 generation', function() {
run_test_keyboard(kmnCompiler, 'test_context_in_context');
});
});


Expand Down

0 comments on commit 186132d

Please sign in to comment.