diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 3564a813c2..c226051ec1 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -96,7 +96,8 @@ define(function (require, exports, module) { EditorPreferences = require("./EditorHelper/EditorPreferences"), ChangeHelper = require("./EditorHelper/ChangeHelper"), ErrorPopupHelper = require("./EditorHelper/ErrorPopupHelper"), - InlineWidgetHelper = require("./EditorHelper/InlineWidgetHelper"); + InlineWidgetHelper = require("./EditorHelper/InlineWidgetHelper"), + ScrollbarHelper = require("./EditorHelper/ScrollbarHelper"); /* Editor preferences */ @@ -408,6 +409,9 @@ define(function (require, exports, module) { // Fail silently; drag image override is non-critical. } + // Click-to-jump on the native scrollbars (instead of slow viewport-at-a-time paging). + ScrollbarHelper.installClickToJump(self); + // Can't get CodeMirror's focused state without searching for // CodeMirror-focused. Instead, track focus via onFocus and onBlur // options and track state with this._focused diff --git a/src/editor/EditorHelper/ScrollbarHelper.js b/src/editor/EditorHelper/ScrollbarHelper.js new file mode 100644 index 0000000000..b675566900 --- /dev/null +++ b/src/editor/EditorHelper/ScrollbarHelper.js @@ -0,0 +1,91 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * Original work Copyright (c) 2012 - 2021 Adobe Systems Incorporated. All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/** + * Editor scrollbar behaviour helpers. Only to be used from Editor.js. + */ + +define(function (require, exports, module) { + + /** + * Install click-to-jump on an editor's native scrollbars: clicking an empty part of the track + * jumps straight to that proportional position, instead of the browser default of paging one + * viewport at a time (painfully slow to reach a far-off spot in a large file). A click on the + * thumb is left to the native drag. + * + * CodeMirror's "native" scrollbars are real overflow:scroll
s (.CodeMirror-vscrollbar / + * .CodeMirror-hscrollbar); setting their scroll offset syncs the editor, since CodeMirror listens + * to their scroll event. Unlike most native scrollbars, this webview still delivers mousedown on + * them, so we can intercept a track click. + * + * @param {!Editor} editor + */ + function installClickToJump(editor) { + const cm = editor._codeMirror; + const wrapper = cm && cm.getWrapperElement(); + if (!wrapper) { + return; + } + + // Capture phase so we can suppress the native paging before it runs. + wrapper.addEventListener("mousedown", function (e) { + const el = e.target; + if (e.button !== 0 || !el || !el.classList) { + return; + } + let axis; + if (el.classList.contains("CodeMirror-vscrollbar")) { + axis = "v"; + } else if (el.classList.contains("CodeMirror-hscrollbar")) { + axis = "h"; + } else { + return; + } + + const rect = el.getBoundingClientRect(); + const view = (axis === "v") ? el.clientHeight : el.clientWidth; // visible track px + const full = (axis === "v") ? el.scrollHeight : el.scrollWidth; // scrollable px + if (full <= view) { + return; // nothing to scroll + } + const cur = (axis === "v") ? el.scrollTop : el.scrollLeft; + const thumbStart = (cur / full) * view; + const thumbSize = (view / full) * view; + const clickPos = (axis === "v") ? (e.clientY - rect.top) : (e.clientX - rect.left); + if (clickPos >= thumbStart && clickPos <= thumbStart + thumbSize) { + return; // on the thumb - let the native drag handle it + } + + // Track click: centre the thumb on the cursor and jump there immediately. + let target = (clickPos / view) * full - view / 2; + target = Math.max(0, Math.min(target, full - view)); + e.preventDefault(); + if (axis === "v") { + el.scrollTop = target; + } else { + el.scrollLeft = target; + } + }, true); + // No explicit removal: the wrapper element is removed on editor.destroy(). + } + + exports.installClickToJump = installClickToJump; +}); diff --git a/src/features/ParameterHintsManager.js b/src/features/ParameterHintsManager.js index 24c004da3a..70c7ddb7e2 100644 --- a/src/features/ParameterHintsManager.js +++ b/src/features/ParameterHintsManager.js @@ -269,8 +269,16 @@ define(function (require, exports, module) { } if (hints.parameters.length > 0) { - let token = editor.getToken(hints.functionCallPos); - _formatParameterHint(token.string, hints.parameters, appendSeparators, appendParameter); + // Prefer a function name supplied by the provider (LSP signatureHelp gives the full + // signature label). Only fall back to the editor token for providers that locate the call + // site themselves (Tern sets functionCallPos); without that fallback guard, an LSP hint - + // whose functionCallPos is undefined - would read the token at the caret, i.e. the just + // typed "(", and render an unbalanced "((". + let functionName = hints.functionName; + if (functionName == null) { + functionName = editor.getToken(hints.functionCallPos).string; + } + _formatParameterHint(functionName, hints.parameters, appendSeparators, appendParameter); } else { $hintContent.append(_.escape(Strings.NO_ARGUMENTS)); } diff --git a/src/languageTools/DefaultProviders.js b/src/languageTools/DefaultProviders.js index 548c598b70..ab0e4d9a66 100644 --- a/src/languageTools/DefaultProviders.js +++ b/src/languageTools/DefaultProviders.js @@ -27,20 +27,22 @@ define(function (require, exports, module) { - var _ = brackets.getModule("thirdparty/lodash"); - - var EditorManager = require('editor/EditorManager'), - DocumentManager = require('document/DocumentManager'), - CommandManager = require("command/CommandManager"), - Commands = require("command/Commands"), - StringMatch = require("utils/StringMatch"), - CodeInspection = require("language/CodeInspection"), - PathConverters = require("languageTools/PathConverters"), - TabstopManager = require("editor/TabstopManager"), - marked = require("thirdparty/marked.min"), - matcher = new StringMatch.StringMatcher({ - preferPrefixMatches: true - }); + var _ = require("thirdparty/lodash"); + + var EditorManager = require("editor/EditorManager"), + DocumentManager = require("document/DocumentManager"), + CommandManager = require("command/CommandManager"), + Commands = require("command/Commands"), + StringMatch = require("utils/StringMatch"), + CodeInspection = require("language/CodeInspection"), + PathConverters = require("languageTools/PathConverters"), + TabstopManager = require("editor/TabstopManager"), + Strings = require("strings"), + StringUtils = require("utils/StringUtils"), + marked = require("thirdparty/marked.min"), + matcher = new StringMatch.StringMatcher({ + preferPrefixMatches: true + }); // Provider styles live in src/styles/brackets.less (core stylesheet) now that languageTools is a // core module - no per-extension stylesheet to load. @@ -151,6 +153,54 @@ define(function (require, exports, module) { } } + // Highlight.js language for the signature block, derived from the file being edited so it isn't + // hard-wired to one language. The LSP `detail` string carries no language tag, so we use the + // editor's. vtsls emits TypeScript-syntax signatures even in .js/.jsx (TS is a superset), so map + // the JS family to ts/tsx; every other language uses its own id - hljs resolves many aliases, and + // _highlightCode no-ops gracefully when the id is unknown (the block just renders as monospace). + var SIGNATURE_HLJS_LANG = { + javascript: "typescript", + jsx: "tsx", + typescript: "typescript", + tsx: "tsx" + }; + + function _signatureLang() { + var editor = EditorManager.getActiveEditor(); + var id = editor && editor.getLanguageForSelection && editor.getLanguageForSelection().getId(); + if (!id) { + return "typescript"; + } + return SIGNATURE_HLJS_LANG[id] || id; + } + + // Build the side doc popup's content, like VS Code/WebStorm's details panel: the item's + // signature (token.detail) as a highlighted code block - the focal point - followed by the + // prose documentation. The signature already carries the "Add import from " line for + // auto-imports (tsserver puts it there), so we don't add our own. Shown whenever there's a + // signature OR docs, so a doc-less item still gets its signature shown instead of nothing. + function _docPopupHtml(token) { + var parts = []; + + if (token.detail) { + try { + parts.push(_highlightCode(marked.parse("```" + _signatureLang() + "\n" + token.detail + "\n```"))); + } catch (e) { + // skip the signature block on any parse/highlight error + } + } + + var docHtml = _docToHtml(token.documentation); + if (docHtml) { + parts.push(docHtml); + } + + if (!token.detail && !docHtml) { + return ""; + } + return parts.join(""); + } + function _showDocPopup($hint, docHtml) { var $menu = $hint.closest(".codehint-menu"); if (!docHtml || !$menu.length) { @@ -235,6 +285,80 @@ define(function (require, exports, module) { return matchResults; } + // True when a completion is an auto-import suggestion (it carries the source module in + // labelDetails.description), as opposed to an in-scope symbol or a keyword. + function _isAutoImport(item) { + return !!(item.labelDetails && item.labelDetails.description); + } + + // WebStorm-style noise control: when several modules export the SAME name, the LSP returns one + // auto-import suggestion per module, flooding the list with near-identical rows. Collapse those + // into a single "N imports…" row; choosing it re-opens the list filtered to just that name's + // sources so the user picks the module (see insertHint + the _importLabel branch of getHints). + // In-scope symbols and single-source auto-imports are left exactly as they were. + function _clubAutoImports(items) { + var counts = new Map(); + items.forEach(function (it) { + if (_isAutoImport(it)) { + counts.set(it.label, (counts.get(it.label) || 0) + 1); + } + }); + var seen = new Set(), result = []; + items.forEach(function (it) { + if (!_isAutoImport(it) || counts.get(it.label) < 2) { + result.push(it); // in-scope, keyword, or the only import source for this name + return; + } + if (seen.has(it.label)) { + return; // already represented by the clubbed row at its best-sorted position + } + seen.add(it.label); + result.push(Object.assign({}, it, { + _clubbed: true, + _importCount: counts.get(it.label) + })); + }); + return result; + } + + // Render one completion row: the (matched-highlighted) label, plus a dimmed right-aligned tag - + // either the source module of a single auto-import, or "N imports…" for a clubbed group. + function _renderHint(element) { + var $fHint = $("").addClass("brackets-hints"); + var $label = $("").addClass("lsp-hint-label"); + + if (element.stringRanges) { + element.stringRanges.forEach(function (item) { + if (item.matched) { + $label.append($("").append(_.escape(item.text)).addClass("matched-hint")); + } else { + $label.append(_.escape(item.text)); + } + }); + } else { + $label.text(element.label); + } + $fHint.append($label); + + if (element._clubbed) { + $("") + .addClass("lsp-hint-source lsp-hint-import-group") + .text(StringUtils.format(Strings.CODE_HINT_IMPORT_FROM_N, element._importCount)) + .appendTo($fHint); + } else if (element.labelDetails && element.labelDetails.description) { + // VS Code-style source annotation: shows e.g. the `inspector` in a `console` auto-import + // so same-named items from different modules read as distinct rows. + $("") + .addClass("lsp-hint-source") + .text(element.labelDetails.description) + .attr("title", element.labelDetails.description) + .appendTo($fHint); + } + + $fHint.data("token", element); + return $fHint; + } + CodeHintsProvider.prototype.hasHints = function (editor, implicitChar) { if (!this.client) { return false; @@ -285,49 +409,41 @@ define(function (require, exports, module) { $deferredHints = $.Deferred(), self = this; - this.client.requestHints({ - filePath: docPath, - cursorPos: pos - }).done(function (msgObj) { - var hints = []; - - // The query is the identifier prefix already typed before the cursor (empty right - // after a trigger char such as "."). Deriving it from the raw token is wrong - after a - // "." the token is "." itself, which would filter out every member completion. + // The query is the identifier prefix already typed before the cursor (empty right after a + // trigger char such as "."). Deriving it from the raw token is wrong - after a "." the token + // is "." itself, which would filter out every member completion. + function computeQuery() { var lineText = editor.document.getLine(pos.line), queryStart = pos.ch; while (queryStart > 0 && /[\w$]/.test(lineText.charAt(queryStart - 1))) { queryStart--; } - self.query = lineText.substring(queryStart, pos.ch); - if (msgObj) { - var res = msgObj.items, - filteredHints = filterWithQueryAndMatcher(res, self.query); + return lineText.substring(queryStart, pos.ch); + } + this.client.requestHints({ + filePath: docPath, + cursorPos: pos + }).done(function (msgObj) { + var hints = []; + self.query = computeQuery(); + if (msgObj) { + var filteredHints = filterWithQueryAndMatcher(msgObj.items, self.query); StringMatch.basicMatchSort(filteredHints); - filteredHints.forEach(function (element) { - var $fHint = $("") - .addClass("brackets-hints"); - - if (element.stringRanges) { - element.stringRanges.forEach(function (item) { - if (item.matched) { - $fHint.append($("") - .append(_.escape(item.text)) - .addClass("matched-hint")); - } else { - $fHint.append(_.escape(item.text)); - } - }); - } else { - $fHint.text(element.label); - } - $fHint.data("token", element); - // The signature is added inline lazily on highlight (onHighlight); the - // documentation is shown in a side popup. See _injectInlineSignature. - hints.push($fHint); - }); + // Second step of a clubbed auto-import: the user picked the "N imports…" row, so show + // just that name's import sources (one row per module) instead of the whole list. + if (self._importLabel) { + var wanted = self._importLabel; + self._importLabel = null; + filteredHints = filteredHints.filter(function (it) { + return it.label === wanted && _isAutoImport(it); + }); + } else { + filteredHints = _clubAutoImports(filteredHints); + } + + hints = filteredHints.map(_renderHint); } $deferredHints.resolve({ @@ -335,6 +451,7 @@ define(function (require, exports, module) { "selectInitial": true }); }).fail(function () { + self._importLabel = null; $deferredHints.reject(); }); @@ -365,6 +482,14 @@ define(function (require, exports, module) { // the list is shown. $hint.closest(".codehint-menu").addClass("lsp-hints"); + // Clubbed "N imports…" row: it stands in for several modules, so resolving its (first + // module's) docs would be misleading. Show a short hint about what selecting it does. + if (token._clubbed) { + _showDocPopup($hint, "

" + StringUtils.format( + Strings.CODE_HINT_IMPORT_CHOOSE, token._importCount, _.escape(token.label)) + "

"); + return; + } + function present() { // Inline: the signature for the highlighted row. Re-inject every time (it is // idempotent) because the list DOM is rebuilt on each keystroke, which drops a @@ -373,8 +498,8 @@ define(function (require, exports, module) { if (token.detail) { _injectInlineSignature($span, token.detail); } - // Beside the list: the (possibly long) documentation. - _showDocPopup($hint, _docToHtml(token.documentation)); + // Beside the list: the signature header + the (possibly long) documentation. + _showDocPopup($hint, _docPopupHtml(token)); } if (token._lspResolved || !self.client.resolveCompletion) { @@ -407,8 +532,21 @@ define(function (require, exports, module) { if (!editor) { return false; } - var token = $hint.data("token") || {}, - cursor = editor.getCursorPos(), + var token = $hint.data("token") || {}; + + // Clubbed "N imports…" row: don't insert anything yet. Re-open the list showing only this + // name's import sources so the user picks the module (WebStorm's "Class to Import" flow). + // getHints honors _importLabel on the next open; the symbol/import is inserted when a + // concrete source is chosen there. + if (token._clubbed) { + this._importLabel = token.label; + setTimeout(function () { + CommandManager.execute(Commands.SHOW_CODE_HINTS); + }, 0); + return false; + } + + var cursor = editor.getCursorPos(), lineText = editor.document.getLine(cursor.line), textEditRange = token.textEdit && token.textEdit.range, startCh, @@ -492,28 +630,35 @@ define(function (require, exports, module) { cursorPos: pos }).done(function (msgObj) { let paramList = []; - let label; - let activeParameter; if (msgObj) { - let res; - res = msgObj.signatures; - activeParameter = msgObj.activeParameter; + let res = msgObj.signatures; + let activeParameter = msgObj.activeParameter; if (res && res.length) { - res.forEach(function (element) { - label = element.documentation; - let param = element.parameters; - param.forEach(ele => { - paramList.push({ - label: ele.label, - documentation: ele.documentation - }); + // Use the active signature (not all of them concatenated - overloads would + // otherwise merge their parameters into one bogus list). + let sig = res[msgObj.activeSignature || 0] || res[0]; + (sig.parameters || []).forEach(function (ele) { + paramList.push({ + label: ele.label, + documentation: ele.documentation }); }); + // The function name for the popup header, taken from the signature label + // (everything before the parameter list "("). Without this the manager falls back + // to the editor token at the caret - which is the just-typed "(" - producing an + // unbalanced "((tableName: any)". + let functionName = ""; + if (typeof sig.label === "string") { + let paren = sig.label.indexOf("("); + functionName = (paren >= 0 ? sig.label.slice(0, paren) : sig.label).trim(); + } + $deferredHints.resolve({ parameters: paramList, currentIndex: activeParameter, - functionDocumentation: label + functionDocumentation: sig.documentation, + functionName: functionName }); } else { $deferredHints.reject(); diff --git a/src/languageTools/LSPClient.js b/src/languageTools/LSPClient.js index 0f39e3f047..6e4441e10a 100644 --- a/src/languageTools/LSPClient.js +++ b/src/languageTools/LSPClient.js @@ -364,6 +364,9 @@ define(function (require, exports, module) { } const signatures = result.signatures.map(function (sig) { return { + // Full signature string (e.g. "getTableIndexes(tableName: any): Promise<…>") - + // the provider derives the function name from it for the parameter-hint popup. + label: sig.label, documentation: _markupToString(sig.documentation) || sig.label, parameters: (sig.parameters || []).map(function (p) { return { @@ -373,7 +376,11 @@ define(function (require, exports, module) { }) }; }); - deferred.resolve({ signatures: signatures, activeParameter: result.activeParameter }); + deferred.resolve({ + signatures: signatures, + activeSignature: result.activeSignature, + activeParameter: result.activeParameter + }); } catch (err) { console.warn("[LSP] request failed:", err && (err.message || err)); deferred.reject(err); @@ -511,7 +518,11 @@ define(function (require, exports, module) { dynamicRegistration: false, completionItem: { snippetSupport: false, - documentationFormat: ["markdown", "plaintext"] + documentationFormat: ["markdown", "plaintext"], + // We render labelDetails.detail/description (e.g. the source module of an + // auto-import) so otherwise-identical labels are distinguishable - see the + // CodeHintsProvider renderer. + labelDetailsSupport: true } }, hover: { dynamicRegistration: false, contentFormat: ["markdown", "plaintext"] }, diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index eef1a315b9..e16d3dfabe 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -1501,6 +1501,8 @@ define({ "CMD_JUMPTO_DEFINITION": "Go to Definition", "CMD_SHOW_PARAMETER_HINT": "Show Parameter Hint", "NO_ARGUMENTS": "", + "CODE_HINT_IMPORT_FROM_N": "{0} imports…", + "CODE_HINT_IMPORT_CHOOSE": "Choose which of {0} modules to import {1} from.", "DETECTED_EXCLUSION_TITLE": "JavaScript File Inference Problem", "DETECTED_EXCLUSION_INFO": "{APP_NAME} ran into trouble processing {0}.

This file will no longer be processed for code hints, Jump to Definition or Quick Edit. To re-enable this file, open .phcode.json in your project and edit jscodehints.detectedExclusions.

This is likely a {APP_NAME} bug. If you can provide a copy of this file, please file a bug with a link to the file named here.", diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 38b97b074f..88f15a97e5 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -2513,12 +2513,9 @@ a, img { } ul, ol { margin: 5px 0; padding-left: 18px; } - &::-webkit-scrollbar { width: 9px; } - &::-webkit-scrollbar-thumb { - background: rgba(0, 0, 0, 0.18); border-radius: 5px; - .dark & { background: rgba(255, 255, 255, 0.18); } - } - &::-webkit-scrollbar-track { background: transparent; } + // No custom scrollbar here: inherit the app-wide one (brackets_scrollbars.less) so this popup's + // scrollbar matches the editor's exactly - the same 12px slim, rounded pill thumb - instead of a + // chunkier block. } // LSP / language-tools code hints. Moved here from languageTools/styles/default_provider_style.css @@ -2637,6 +2634,14 @@ span.brackets-hints-with-type-details { } .codehint-menu.lsp-hints .codehint-item > .brackets-hints { + flex: 1 1 auto; + min-width: 0; + display: flex; + align-items: center; +} + +// The label itself takes the flexible space and truncates; the source tag stays whole. +.codehint-menu.lsp-hints .lsp-hint-label { flex: 1 1 auto; min-width: 0; overflow: hidden; @@ -2644,6 +2649,34 @@ span.brackets-hints-with-type-details { white-space: nowrap; } +// Source module of an auto-import (labelDetails.description), dimmed at the right edge - VS Code's +// way of distinguishing same-named completions from different modules. +.codehint-menu.lsp-hints .lsp-hint-source { + flex: 0 0 auto; + margin-left: 10px; + max-width: 50%; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: 11px; + font-style: italic; + opacity: 0.55; +} + +// On the highlighted row the side doc popup already names the import source, and the inline +// signature takes the right slot, so hide the source tag there to keep the row uncluttered. +// The clubbed "N imports…" tag is the row's whole purpose, so keep it visible when highlighted. +.codehint-menu.lsp-hints .highlight .lsp-hint-source:not(.lsp-hint-import-group) { + display: none; +} + +// The clubbed "N imports…" affordance reads as an action, not a source path: not italic, a touch +// more present than a plain source tag. +.codehint-menu.lsp-hints .lsp-hint-import-group { + font-style: normal; + opacity: 0.75; +} + .lsp-hint-sig { display: none; }