Skip to content
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

257 autocomplete options in flowr repl #630

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/cli/common/scripts-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export type ScriptInformation = MasterScriptInformation | HelperScriptInformatio
/**
* We hold `_scripts` internally, as the modifiable variant and export the readonly scripts
*/
const _scripts = {
const _scripts: Record<string, ScriptInformation> = {
'slicer': {
toolName: 'slicer',
target: 'slicer-app',
Expand Down Expand Up @@ -97,5 +97,8 @@ const _scripts = {
}
}

export const scripts = _scripts as Record<keyof typeof _scripts, ScriptInformation>
export const scripts = _scripts satisfies Record<keyof typeof _scripts, ScriptInformation>

export function getScriptInformation(scriptName: string): ScriptInformation | undefined{
return scripts[scriptName]
}
87 changes: 86 additions & 1 deletion src/cli/repl/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,103 @@ import { prompt } from './prompt'
import type { ReplOutput} from './commands'
import { commandNames, getCommand, standardReplOutput } from './commands'
import * as readline from 'node:readline'
import { splitAtEscapeSensitive } from '../../util/args'
import { splitAtEscapeSensitive, splitAtEscapeSensitiveWithEmptyParameters } from '../../util/args'
import { executeRShellCommand } from './commands/execute'
import { guard } from '../../util/assert'
import { getScriptInformation} from '../common/scripts-info'
import type { OptionDefinition } from 'command-line-usage'

const replCompleterKeywords = Array.from(commandNames, s => `:${s}`)

/**
* Used by the repl to provide automatic completions for a given (partial) input line
*/
export function replCompleter(line: string): [string[], string] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests hierfür wären bestimmt gut

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

du meinst für den replCompleter? (Unklar weil der Kommentar auch zu den "import"-Statements ist)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

die sind nur dabei weil GitHub immer kontext liefert, es ist die unterste Zeile :D also ja für den completer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vielleicht gleich einen Ort spezifizieren wo sie reinkommen sollten (also datei angeben und ob die neu erstellt werden muss)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test/functionality/cli/repl.ts

cli und repl müssen neu erstellt werden

const splitCommandList = splitAtEscapeSensitiveWithEmptyParameters(line)
//find command in commandlist
const keyword = replCompleterKeywords.find(k => splitCommandList[0] === k)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this check we are probably better off with a set, given that more and more commands are added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for several keywords to be exactly equal?
In this instance it can only be one keyword. Shouldn't multiple commands never be mapped to one keyword

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my comment was about the speed - with find you do a linear search on all keywords. for the autocompletion to be faster, with more and more commands, i think a set would be fine because we only check if a given command exists.

if(keyword !== undefined && keyword.startsWith(':')){
const singleCommand = getCommand(keyword.slice(1))
guard(singleCommand !== undefined, 'Keyword does not match any command')
if(singleCommand.script){
const scriptInformation = getScriptInformation(keyword.slice(1))
guard(scriptInformation !== undefined, 'script should be in script record')
const scriptOptions: OptionDefinition[] = scriptInformation.options
const possibleOptions :OptionDefinition[] = getPossibleCommandLineOptions(scriptOptions, splitCommandList)
const possibleCliOptionsAsString : string[] = extractCliStringsFromCommandOptions(possibleOptions)
//Only command was specified
if(splitCommandList.length < 2){
possibleCliOptionsAsString.push(' ')
return [possibleCliOptionsAsString, line]
}

const lastOptionInProgress = splitCommandList.at(-1)
guard(lastOptionInProgress !== undefined, 'splitCommandList cannot be of lenth 0')
const matchingOptionsLeft = possibleCliOptionsAsString.filter(o => o.startsWith(lastOptionInProgress))
if(matchingOptionsLeft.length === 0){
return [[], line]
}
if(matchingOptionsLeft.length === 1){
const commandWithoutLastEntry = splitCommandList.slice(0, -1).join(' ')
return [[commandWithoutLastEntry + ' ' + matchingOptionsLeft[0]], line]
}
matchingOptionsLeft.push(' ')
return [matchingOptionsLeft, line]
}
}
return [replCompleterKeywords.filter(k => k.startsWith(line)), line]
}

export function extractCliStringsFromCommandOptions(options: OptionDefinition[]):string[]{
const extractedCliStrings:string[] = []
options.forEach(o => {
extractedCliStrings.push('--' + o.name)
if(o.alias !== undefined){
extractedCliStrings.push('-' + o.alias)
}
})
return extractedCliStrings
}

function getPossibleCommandLineOptions(scriptOptions: OptionDefinition[], splitCommandList: string[]): OptionDefinition[] {
const optionUsed: Map<string, boolean> = new Map <string, boolean>()
scriptOptions.forEach(o => optionUsed.set(o.name, false))
for(let commandLineOptionIndex = 1; commandLineOptionIndex < splitCommandList.length; commandLineOptionIndex++){
if(splitCommandList[commandLineOptionIndex].substring(0,1) !== '-'){
continue
}
let cliOptionWithOutLeadingMinus: string = splitCommandList[commandLineOptionIndex].slice(1)
if(cliOptionWithOutLeadingMinus.substring(0,1) !== '-'){
const option = scriptOptions.find(o => o.alias === cliOptionWithOutLeadingMinus)
if(option !== undefined){
optionUsed.set(option.name, true)
} else {
continue
}
}

cliOptionWithOutLeadingMinus = cliOptionWithOutLeadingMinus.slice(1)
if(cliOptionWithOutLeadingMinus.substring(0,1) !== '-'){
if(optionUsed.has(cliOptionWithOutLeadingMinus)){
optionUsed.set(cliOptionWithOutLeadingMinus, true)
}
}
}
const possibleOptions : OptionDefinition[] = []
optionUsed.forEach((usedValue, optionName) => {
const newOption = scriptOptions.find(o => o.name === optionName)
guard(newOption !== undefined, 'Option contained must be in original Options list')
if(usedValue){
if(newOption.multiple !== undefined && newOption.multiple){
possibleOptions.push(newOption)
}
} else {
possibleOptions.push(newOption)
}
})
return possibleOptions
}

export const DEFAULT_REPL_READLINE_CONFIGURATION: readline.ReadLineOptions = {
input: process.stdin,
output: process.stdout,
Expand Down
49 changes: 49 additions & 0 deletions src/util/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,52 @@ export function splitAtEscapeSensitive(inputString: string, split = ' '): string

return args
}

/**
* This splits an input string on the given split string (e.g., ` `) but checks if the string is quoted or escaped.
* Also takes into account if you have the splitsymbol at the start and e.g `' '` converts to `['', '', '']` and `' a "b c" d '` converts to `['', 'a', 'b c', 'd', '']`
* Given an input string like `a "b c" d` with a space character as split this splits the arguments similar to common shell interpreters (i.e., `a`, `b c`, and `d`).
*
* @param inputString - The string to split
* @param split - The **single** character to split on (can not be the backslash or quote)
*/
export function splitAtEscapeSensitiveWithEmptyParameters(inputString: string, split = ' '): string[] {
const args = []
let current = ''
let inQuotes = false
let escaped = false

for(const c of inputString) {
if(escaped) {
escaped = false
switch(c) {
case 'n': current += '\n'; break
case 't': current += '\t'; break
case 'r': current += '\r'; break
case 'v': current += '\v'; break
case 'f': current += '\f'; break
case 'b': current += '\b'; break
default: current += c
}
} else if(c === split && !inQuotes) {
args.push(current)
current = ''
} else if(c === '"' || c === "'") {
inQuotes = !inQuotes
} else if(c === '\\') {
escaped = true
} else {
current += c
}
}

if(current !== '') {
args.push(current)
}

if(inputString.slice(-1) === ' '){
args.push('')
}

return args
}
3 changes: 3 additions & 0 deletions test/functionality/cli/cli.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
describe('CLI', () => {
require('./repl-tests')
})
26 changes: 26 additions & 0 deletions test/functionality/cli/repl-tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { assert } from 'chai'
import { replCompleter } from '../../../src/cli/repl'
import { benchmarkOptions } from '../../../src/cli/common/options'

describe('Repl', () => {
describe('completer', () => {
describe('basic find', () =>
assert.deepStrictEqual([[':slicer'], ':slice'], replCompleter(':slice'), 'standard replCompleter not producing expected result')

)
describe('find all options', () => {
const optionsList: string[] = []
for(const [,{name, alias}] of Object.entries(benchmarkOptions)){
optionsList.push('--' + name)
if(alias !== undefined){
optionsList.push('-' + alias)
}
}
optionsList.push(' ')
const resultCompleter = replCompleter(':benchmark ')
assert.includeMembers(resultCompleter[0], optionsList, 'Additional options that should not be there')
assert.includeMembers(optionsList, resultCompleter[0], 'Options missing in the expected Result')
assert.strictEqual(resultCompleter[1], ':benchmark ')
})
})
})
Loading