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

web/settings: Fix quick action selection #32258

Closed
wants to merge 4 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 37 additions & 13 deletions client/web/src/settings/MonacoSettingsEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ export class MonacoSettingsEditor extends React.PureComponent<Props, State> {
const { edits, selectText, cursorOffset } = run(editor.getValue())
const monacoEdits = toMonacoEdits(model, edits)
let selection: monaco.Selection | undefined
const afterText = jsonc.applyEdits(editor.getValue(), edits)

if (typeof selectText === 'string') {
const afterText = jsonc.applyEdits(editor.getValue(), edits)
let offset = afterText.slice(edits[0].offset).indexOf(selectText)
if (offset !== -1) {
offset += edits[0].offset
Expand All @@ -242,18 +243,41 @@ export class MonacoSettingsEditor extends React.PureComponent<Props, State> {
}
}
if (!selection) {
// TODO: This is buggy. See
// https://github.com/sourcegraph/sourcegraph/issues/2756.
selection = monaco.Selection.fromPositions(
{
lineNumber: monacoEdits[0].range.startLineNumber,
column: monacoEdits[0].range.startColumn,
},
{
lineNumber: monacoEdits[monacoEdits.length - 1].range.endLineNumber,
column: monacoEdits[monacoEdits.length - 1].range.endColumn,
}
)
// `jsonc-parser.modify()` returns a `jsonc-parser.Edit` which
// includes the line before the added property (because the edit
// involves comma+line-separation and formatting)
//
// Assumptions:
// 1. Well-formed JSON
// 2. `formattingOptions.eol` in `configHelpers.ts` is set to `'\n'`
// 3. There's always a preceding line, eg:
// - `{\n...` (empty settings object)
// - ` "search.contextLines": 3,\n...` (previous property is a scalar)
// - ` ],\n...` (previous property is an array)

const insertionWithPrecedingLine = edits[0].content
const insertionLineOffset = insertionWithPrecedingLine.indexOf('\n') + 1
const insertion = insertionWithPrecedingLine.slice(insertionLineOffset)
const reProperty = /"[^"]+"\s*:\s*/g; // Need global flag `g` set to get `lastIndex`
// Need semi-colon after regex flags
const match = reProperty.exec(insertion)
if (!match) {
throw new Error("Can't find property name in added JSON. This should never happen.");
}
const valueOffset = insertionLineOffset + reProperty.lastIndex
const offset = edits[0].offset + valueOffset
const length = edits[0].content.length - valueOffset
if (typeof cursorOffset === 'number') {
selection = monaco.Selection.fromPositions(
getPositionAt(afterText, offset + cursorOffset),
getPositionAt(afterText, offset + cursorOffset)
)
} else {
selection = monaco.Selection.fromPositions(
getPositionAt(afterText, offset),
getPositionAt(afterText, offset + length)
)
}
}
editor.executeEdits(id, monacoEdits, [selection])
editor.revealPositionInCenter(selection.getStartPosition())
Expand Down
15 changes: 8 additions & 7 deletions client/web/src/site-admin/configHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
import { FormattingOptions } from '@sqs/jsonc-parser'
import { setProperty } from '@sqs/jsonc-parser/lib/edit'
import { FormattingOptions , modify } from '@sqs/jsonc-parser'

import { ConfigInsertionFunction } from '../settings/MonacoSettingsEditor'

const defaultFormattingOptions: FormattingOptions = {
const formattingOptions: FormattingOptions = {
eol: '\n',
insertSpaces: true,
tabSize: 2,
}

const options = { formattingOptions }

const setSearchContextLines: ConfigInsertionFunction = config => {
const DEFAULT = 3 // a reasonable value that will be clearly different from the default 1
return { edits: setProperty(config, ['search.contextLines'], DEFAULT, defaultFormattingOptions) }
return { edits: modify(config, ['search.contextLines'], DEFAULT, options) }
}

const addSearchScopeToSettings: ConfigInsertionFunction = config => {
const value: { name: string; value: string } = {
name: '<name>',
value: '<partial query string that will be inserted when the scope is selected>',
}
const edits = setProperty(config, ['search.scopes', -1], value, defaultFormattingOptions)
const edits = modify(config, ['search.scopes', -1], value, options)
return { edits, selectText: '<name>' }
}

const addQuickLinkToSettings: ConfigInsertionFunction = config => {
const value: { name: string; url: string } = {
name: '<human-readable name>',
name: '<name>',
url: '<URL>',
}
const edits = setProperty(config, ['quicklinks', -1], value, defaultFormattingOptions)
const edits = modify(config, ['quicklinks', -1], value, options)
return { edits, selectText: '<name>' }
}

Expand Down