Skip to content

Commit

Permalink
Unify the two copies of cleanKind
Browse files Browse the repository at this point in the history 
So the code coverage does not drop too much.

At the same time, it's clear the heuristic used is pretty gross,
and we should use the #_[canonical] annotation on the class instead.
  • Loading branch information
@gcampax
gcampax committed Nov 1, 2019
1 parent 883d839 commit e9fa3a9
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 50 deletions.
24 changes: 1 addition & 23 deletions lib/ast/class_def.js
View file
Expand Up @@ -14,7 +14,7 @@ const assert = require('assert');
//const Ast = require('./ast');
const Type = require('../type');
const { prettyprintClassDef } = require('../prettyprint');
const { clean } = require('../utils');
const { clean, cleanKind } = require('../utils');
const { Value } = require('./values');
const { Statement, InputParam } = require('./program');
const { FunctionDef } = require('./function_def');
Expand All @@ -23,28 +23,6 @@ const toJS = require('./toJS');

// Class definitions

function cleanKind(kind) {
// thingengine.phone -> phone
if (kind.startsWith('org.thingpedia.builtin.thingengine.'))
kind = kind.substr('org.thingpedia.builtin.thingengine.'.length);
// org.thingpedia.builtin.omlet -> omlet
if (kind.startsWith('org.thingpedia.builtin.'))
kind = kind.substr('org.thingpedia.builtin.'.length);
// org.thingpedia.weather -> weather
if (kind.startsWith('org.thingpedia.'))
kind = kind.substr('org.thingpedia.'.length);
// com.xkcd -> xkcd
if (kind.startsWith('com.'))
kind = kind.substr('com.'.length);
if (kind.startsWith('gov.'))
kind = kind.substr('gov.'.length);
if (kind.startsWith('org.'))
kind = kind.substr('org.'.length);
if (kind.startsWith('uk.co.'))
kind = kind.substr('uk.co.'.length);
return clean(kind);
}

/**
* The definition of a ThingTalk class.
*
Expand Down
29 changes: 6 additions & 23 deletions lib/describe.js
View file
Expand Up @@ -13,7 +13,7 @@ const assert = require('assert');

const Ast = require('./ast');
const Type = require('./type');
const { clean } = require('./utils');
const { clean, cleanKind } = require('./utils');
const FormatUtils = require('./format_utils');
const { Currency } = require('./builtin/values');
const I18n = require('./i18n');
Expand Down Expand Up @@ -856,9 +856,9 @@ class Describer {

switch (functionType) {
case 'query':
return this._("your %s").format(doCapitalizeSelector(kind));
return this._("your %s").format(capitalize(cleanKind(kind)));
case 'action':
return this._("perform any action on your %s").format(doCapitalizeSelector(kind));
return this._("perform any action on your %s").format(capitalize(cleanKind(kind)));
default:
return '';
}
Expand Down Expand Up @@ -981,7 +981,7 @@ class Describer {
}

function capitalize(str) {
return (str[0].toUpperCase() + str.substr(1)).replace(/[.\-_]([a-z])/g, (whole, char) => ' ' + char.toUpperCase()).replace(/[.\-_]/g, '');
return str.split(/\s+/g).map((word) => word[0].toUpperCase() + word.substring(1)).join(' ');
}

function capitalizeSelector(prim) {
Expand All @@ -994,27 +994,10 @@ function capitalizeSelector(prim) {
}

function doCapitalizeSelector(kind, channel) {
// thingengine.phone -> phone
if (kind.startsWith('org.thingpedia.builtin.thingengine.'))
kind = kind.substr('org.thingpedia.builtin.thingengine.'.length);
// org.thingpedia.builtin.omlet -> omlet
if (kind.startsWith('org.thingpedia.builtin.'))
kind = kind.substr('org.thingpedia.builtin.'.length);
// org.thingpedia.weather -> weather
if (kind.startsWith('org.thingpedia.'))
kind = kind.substr('org.thingpedia.'.length);
// com.xkcd -> xkcd
if (kind.startsWith('com.'))
kind = kind.substr('com.'.length);
if (kind.startsWith('gov.'))
kind = kind.substr('gov.'.length);
if (kind.startsWith('org.'))
kind = kind.substr('org.'.length);
if (kind.startsWith('uk.co.'))
kind = kind.substr('uk.co.'.length);
kind = cleanKind(kind);

if (kind === 'builtin' || kind === 'remote' || kind.startsWith('__dyn_'))
return capitalize(channel);
return capitalize(clean(channel));
else
return capitalize(kind);
}
Expand Down
24 changes: 24 additions & 0 deletions lib/utils.js
View file
Expand Up @@ -23,6 +23,29 @@ function clean(name) {
return name.replace(/_/g, ' ').replace(/([^A-Z ])([A-Z])/g, '$1 $2').toLowerCase();
}

function cleanKind(kind) {
// thingengine.phone -> phone
if (kind.startsWith('org.thingpedia.builtin.thingengine.'))
kind = kind.substr('org.thingpedia.builtin.thingengine.'.length);
// org.thingpedia.builtin.omlet -> omlet
if (kind.startsWith('org.thingpedia.builtin.'))
kind = kind.substr('org.thingpedia.builtin.'.length);
// org.thingpedia.weather -> weather
if (kind.startsWith('org.thingpedia.'))
kind = kind.substr('org.thingpedia.'.length);
// com.xkcd -> xkcd
if (kind.startsWith('com.'))
kind = kind.substr('com.'.length);
if (kind.startsWith('gov.'))
kind = kind.substr('gov.'.length);
if (kind.startsWith('org.'))
kind = kind.substr('org.'.length);
if (kind.startsWith('uk.co.'))
kind = kind.substr('uk.co.'.length);
kind = kind.replace(/[.-]/g, ' ');
return clean(kind);
}

// this regexp is similar to the one in runtime/formatter.js, but it does not allow '%' as an option
// FIXME: unify
const PARAM_REGEX = /\$(?:\$|([a-zA-Z0-9_]+(?![a-zA-Z0-9_]))|{([a-zA-Z0-9_]+)(?::([a-zA-Z0-9_]+))?})/;
Expand Down Expand Up @@ -52,6 +75,7 @@ module.exports = {
split,
makeIndex,
clean,
cleanKind,

getSchemaForSelector(schemaRetriever, type, name, schemaType, getMeta = false, classes = {}) {
if (type in classes) {
Expand Down
8 changes: 4 additions & 4 deletions test/test_describe.js
View file
Expand Up @@ -305,13 +305,13 @@ var TEST_CASES = [
`tweet the selection on the screen`, `Twitter`],

['now => @light-bulb.set_power();',
'turn ____ your light-bulb', 'Light Bulb'],
'turn ____ your light bulb', 'Light Bulb'],
['now => @light-bulb(name="bedroom").set_power();',
'turn ____ your “bedroom” light-bulb', 'Light Bulb'],
'turn ____ your “bedroom” light bulb', 'Light Bulb'],
['now => @light-bulb(name="bedroom", all=true).set_power();',
'turn ____ all your “bedroom” light-bulb', 'Light Bulb'],
'turn ____ all your “bedroom” light bulb', 'Light Bulb'],
['now => @light-bulb(all=true).set_power();',
'turn ____ all your light-bulb', 'Light Bulb'],
'turn ____ all your light bulb', 'Light Bulb'],

[`monitor (@smoke-alarm.status()) => notify;`,
'notify you when the status of your smoke alarm changes', 'Smoke Alarm ⇒ Notification'],
Expand Down

0 comments on commit e9fa3a9

Please sign in to comment.