triggering a param add pop-up if running a cypher with missing params #452
Conversation
|
| export function extractParameters(strings: string[]): string[] { | ||
| const paramSet = new Set<string>(); | ||
|
|
||
| strings.forEach((str) => { | ||
| const matches = str.match(/\$`([^`]+)`|\$(\w+)/g); | ||
| if (matches) { | ||
| matches.forEach((match) => { | ||
| // Remove the '$' | ||
| const param = match.slice(1); | ||
| if (param.startsWith('`') && param.endsWith('`')) { | ||
| // Remove the backticks | ||
| paramSet.add(param.slice(1, -1)); | ||
| } else { | ||
| paramSet.add(param); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| return Array.from(paramSet); | ||
| } |
There was a problem hiding this comment.
I think it'd make more sense to have a small method in the language support that returns the parameters using the parser instead?
I see you've added the collectedParameters: ParsedParameter[] in #456 so it'd make sense to expose that?
There was a problem hiding this comment.
Makes sense! I will continue with this pr after I merge #456 and try to use collectedParameters instead of this method.
There was a problem hiding this comment.
I've changed the implementation to call createParsingResult, to get the collectedParameters. I'm not sure if it is a too costly operation to be honest, and if I should add a new method only to collect the parameters for this purpose. What do you think?
| @@ -7,7 +7,12 @@ export { _internalFeatureFlags } from './featureFlags'; | |||
| export { formatQuery } from './formatting/formatting'; | |||
| export { antlrUtils } from './helpers'; | |||
| export { CypherTokenType, lexerSymbols } from './lexerSymbols'; | |||
| export { parse, parserWrapper, parseStatementsStrs } from './parserWrapper'; | |||
| export { | |||
| createParsingResult, | |||
There was a problem hiding this comment.
Maybe it'd be better to have a parseParameters method in the language support rather than exposing this internal?
There was a problem hiding this comment.
Changed it to have a parseParameters which returns an array of strings(cleared param names)
| async function escapeModal(count: number) { | ||
| for (let i = 0; i < count; i++) { | ||
| await browser.pause(500); | ||
| await browser.keys([Key.Escape]); | ||
| await waitUntilNotification(browser, 'Parameter value cannot be empty.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Was this flaky when you just did waitUntilNotification without browser.pause, or why was this added? In general it feels a little sketchy to use browser.pause() but in some cases it seems hard to avoid. (For example with waiting for the inputBox to open to type things into it)
There was a problem hiding this comment.
Yes, actually, it was flaky. So, I'd like to keep these just in case, so that the browser wouldn't click on the Escape button before the parameter add pop-up opens :/
This pr is to trigger the parameter add pop-up when a user wants to run a query with missing parameters. With these changes:
promptawaittovoidto prevent the wait for user to manually close the error message before opening the add pop-up for the following missing parameterextractParameterswhich gets an array of strings(queries) and returns distinct paramNames