-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add additional ${name:func} substitutions for labelFormat #447
Conversation
Hi, I used "TAG" in case that you also checked for a "subtag" and "subTag" replacement. |
P.S.: Sorry for adding line endings to the code - that's happened while editing in GitHub browser editor. |
Hi Nigel ( @Gruntfuggly ) I "repaired" the additional file ends and squashed the changes. If anything left just leave a note. Cheers |
Hi - thanks for this. I was going to merge it, but then I wondered whether it should apply similar rules to other substitutions? I quite like the idea of allowing uppercase, lowercase and possibly capitalize for tag and the subTag too. Your pattern of using TAG would have been fine if I hadn't added How would you feel about adding ${tag:uppercase} It probably needs to be refactored into a function. I'm happy to make the change myself, unless you want to, or you have any other ideas? |
Hi Nigel
sounds good, I will make a proposal tonight.
cheers
Tom
|
Hi Nigel @Gruntfuggly what about rewriting the function like this: function formatLabel( template, node, unexpectedPlaceholders )
{
var result = template;
var tag = String(node.actualTag).trim();
var subTag = node.subTag ? String(node.subTag).trim() : "";
var filename = node.fsPath ? path.basename( node.fsPath ) : "";
var filepath = node.fsPath ? node.fsPath : "";
var formatLabelMap = {
"line": function() { return node.line + 1; },
"column": function() { return node.column; },
"tag": function() { return tag; },
"tag:uppercase": function() { return tag.toUpperCase(); },
"tag:lowercase": function() { return tag.toLowerCase(); },
"tag:capitalize": function() { return tag.charAt(0).toUpperCase() + tag.slice(1); },
"subtag": function() { return subTag; },
"subtag:uppercase": function() { return subTag.toUpperCase(); },
"subtag:lowercase": function() { return subTag.toLowerCase(); },
"subtag:capitalize": function() { return ( subTag === "" ) ? "" : subTag.charAt(0).toUpperCase() + subTag.slice(1); },
"before": function() { return node.before; },
"after": function() { return node.after; },
"afterOrBefore": function() { return ( node.after === "" ) ? node.before : node.after; },
"filename": function() { return filename; },
"filepath": function() { return filepath }
}
// prepare regex to substitude "${name}" with it's function return value
var re = new RegExp("\\$\\{(" + Object.keys(formatLabelMap).join("|") + ")\\}", "gi");
result = result.replace(re, function(matched){
return formatLabelMap[matched.slice(2, -1).toLowerCase()]();
});
if( unexpectedPlaceholders )
{
var placeholderMatch = placeholderRegex.exec( result );
if( placeholderMatch )
{
unexpectedPlaceholders.push( placeholderMatch[ 0 ] );
}
}
return result;
} I use anonymous functions as returns in case of performance but not sure about that and if necessary. If this map will be used often - maybe its a speed improvement not to build all the values all the time? What do you think? This variant will work also: function formatLabel( template, node, unexpectedPlaceholders )
{
var result = template;
var tag = String(node.actualTag).trim();
var subTag = node.subTag ? String(node.subTag).trim() : "";
var filename = node.fsPath ? path.basename( node.fsPath ) : "";
var filepath = node.fsPath ? node.fsPath : "";
var formatLabelMap = {
"line": node.line + 1,
"column": node.column,
"tag": tag,
"tag:uppercase": tag.toUpperCase(),
"tag:lowercase": tag.toLowerCase(),
"tag:capitalize": tag.charAt(0).toUpperCase() + tag.slice(1),
"subtag": subTag,
"subtag:uppercase": subTag.toUpperCase(),
"subtag:lowercase": subTag.toLowerCase(),
"subtag:capitalize": ( subTag === "" ) ? "" : subTag.charAt(0).toUpperCase() + subTag.slice(1),
"before": node.before,
"after": node.after,
"afterOrBefore": ( node.after === "" ) ? node.before : node.after,
"filename": filename,
"filepath": filepath
}
// prepare regex to substitude "${name}" with it's value
var re = new RegExp("\\$\\{(" + Object.keys(formatLabelMap).join("|") + ")\\}", "gi");
result = result.replace(re, function(matched){
return formatLabelMap[matched.slice(2, -1).toLowerCase()];
});
if( unexpectedPlaceholders )
{
var placeholderMatch = placeholderRegex.exec( result );
if( placeholderMatch )
{
unexpectedPlaceholders.push( placeholderMatch[ 0 ] );
}
}
return result;
} What would you prefer and decide? |
Hey Nigel @Gruntfuggly I have prepared a benchmark codepen at https://codepen.io/TomFreudenberg/pen/abBGjjb What you will get is testing of 2 templates running against formatLabel() with anonymous functions and / or values var template_min = "${tag} : ${after}"
var template_max = "${tag:uppercase}[${line}] : ${subTag:lowercase} ${after}" The result is looking like: Results Tests are running ... Anonymous Func Min x 709,300 ops/sec ±0.92% (64 runs sampled) Anonymous Func Max x 445,141 ops/sec ±0.65% (62 runs sampled) Values Min x 582,359 ops/sec ±0.65% (62 runs sampled) Values Max x 417,058 ops/sec ±0.76% (62 runs sampled) Fastest is Anonymous Func Min That does mean: If the template will just substitude a few variables then anonymous functions are faster but if there are more replacements in the templates the values-way is faster. What is your decision? |
P.S.: Hopefully you are also fine too look for case insensitive ${names} so subtag, subTag, SubTag ... is all the same? |
Last but not least - ES6 style is looking "nice" to read in source code var formatLabelMap = {
"line": () => node.line + 1,
"column": () => node.column,
"tag": () => tag,
"tag:uppercase": () => tag.toUpperCase(),
"tag:lowercase": () => tag.toLowerCase(),
"tag:capitalize": () => tag.charAt(0).toUpperCase() + tag.slice(1),
"subtag": () => subTag,
"subtag:uppercase": () => subTag.toUpperCase(),
"subtag:lowercase": () => subTag.toLowerCase(),
"subtag:capitalize": () => ( subTag === "" ) ? "" : subTag.charAt(0).toUpperCase() + subTag.slice(1),
"before": () => node.before,
"after": () => node.after,
"afterOrBefore": () => ( node.after === "" ) ? node.before : node.after,
"filename": () => filename,
"filepath": () => filepath
} Results Tests are running ... TODO[1] : herowork my description Anonymous Func Min x 700,944 ops/sec ±0.64% (63 runs sampled) Anonymous Func Max x 448,634 ops/sec ±0.87% (62 runs sampled) ES6 Func Min x 690,559 ops/sec ±0.81% (63 runs sampled) ES6 Func Max x 444,293 ops/sec ±0.89% (64 runs sampled) Values Min x 604,301 ops/sec ±0.82% (60 runs sampled) Values Max x 456,359 ops/sec ±0.82% (65 runs sampled) Fastest is Anonymous Func Min As overall - I would stay with "the values" |
Substitute of: ${tag:uppercase} ${tag:lowercase} ${tag:capitalize} ${subtag:uppercase} ${subtag:lowercase} ${subtag:capitalize} Additional tests for ${name:function} substitutions
Hi Nigel, I updated the PR and squashed the changes - looking forward to your feedback if that fit's Cheers |
Hi Tom - looks good at first glance. I'll have a proper look tomorrow - I've been cutting a sofa in to pieces to get it up the stairs into my daughters bedroom. 🙄 |
Hi Nigel @Gruntfuggly just give me a note if something left todo. Cheers |
Looks great - thanks! I'll do a release tonight. |
This PR makes it possible to scan for upperCase and lowerCase tags or case insensitive but to show tag labels in a general upperCase manner inside the tree views.
Makes it much more easier to get through the tree views.