Add language icon, New File entry, and .sql file support - #502
Open
juliasilge wants to merge 7 commits into
Open
Add language icon, New File entry, and .sql file support#502juliasilge wants to merge 7 commits into
juliasilge wants to merge 7 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR provides three extension improvements, all of which are focused on improving discoverability. Two of the three issues above describe the current situation slightly inaccurately, so this PR does not implement them literally.
This PR also fixes a pre-existing bug where plain VS Code offered run actions that had no handler. That is the last section down there at the bottom.
posit-dev/positron#14953: language icon missing
It turns out that
contributes.languages[].iconpointed at./icon.png, which does not exist and never has; the only images shipped arelogo.pngandresources/ggsql-icon.svg.Nothing validates that path, so
languageServicehanded back a URI to a missing file and the icon theme emitted a rule withcontent: '\2001'(an em quad, used so image icons do not collapse to zero height) plus abackground-imagethat 404s. The result is correctly sized blank space. The broken contribution was worse than no contribution at all. Without it, the item would have fallen through to Seti's generic file glyph, but the language-icon rule is more specific and won.This PR adds a new
resources/ggsql-lang-icon.svgthat is a single flat#0A9396fill with transparent seams, following the file-icon-theme idiom where the background shows through the cuts rather than a second colour being painted. One geometry therefore serves light and dark themes, andicon.lightandicon.darkpoint at the same file.#0A9396is 3.73:1 on white and 4.42:1 on#1f1f1f, so it clears the 3:1 floor for non-text graphics on both; the dark and light teals from the logo each fail on one background, which is why a single file works and a colour pair would not.The geometry is derived from the logo rather than redrawn: a boolean union of its own path data, preserving the 29 degree tilt, with three seams subtracted on the logo's own band boundaries. Ink spans 87.5% of a square viewBox, landing at 14px in the 16px slot to match the
positron-rreference icon's weight. The viewBox is centred on the shape's area centroid rather than its bounding box, because the thick body sits up-right while the point trails down-left, which made the icon read as shifted left.This problem was actually broader than the issue reports. Both surfaces named in the issue, the interpreter selector and the console pane, resolve icons through
getSessionIconClassesand so are both fixed. The same broken rule was also suppressing the icon for.gsqlfiles in the Explorer, editor tabs and breadcrumbs, which the issue does not mention.resources/ggsql-icon.svgis untouched, sincemanager.tsstill reads it forbase64EncodedIconSvg.Here are some screenshots of how this looks now, in various parts of the IDE:
It does not change the parts of the IDE where we use the "real" logo:
posit-dev/positron#14955: ggsql file type in New File
Adds a
ggsql.createNewFilecommand contributed to thefile/newFilemenu, so New File... offers "ggsql File". It opens an untitled document with theggsqllanguage, matching howpositron-randpositron-pythonand many other extensions implement the same contribution.FYI the issue and ggsql's own docs use
.gsqlas the main extension, and that needed a manifest change to actually work.contributes.languages[].extensionslisted.ggsqlfirst, andtextFileService.suggestFilenameusesextensions.at(0), so saving an untitled ggsql document suggestedUntitled-1.ggsql. The array is reordered to put.gsqlfirst, which also matches the docs, which use.gsqleverywhere a real file extension appears, and never.ggsql. All three extensions stay registered, so existing files keep working.Unlike
positron-randpositron-python, which usegroup: "file", this uses its ownggsqlgroup so the entry appears under its own heading like Quarto rather than mixed in with the built-in file types, reflecting that ggsql is not built into Positron:posit-dev/positron#14971: using
.sqlfiles with ggsqlFYI the issue's premise conflates two separate things, and the more useful fix is not the one it proposes. It states that running code in a
.sqlfile with a ggsql kernel requires settingfiles.associations. Execution never actually required that.executeCode('ggsql', ...)names the runtime explicitly, so the document's language id is irrelevant to running the code; the setting was only needed because the extension gated its own run affordances onresourceLangId == ggsql.So rather than making the association easier to reach, this removes the need for it. The "Source Current File" button, the code cells and the CodeLens now attach to
sqldocuments as well asggsqlones. Running SQL in a ggsql console works out of the box, with no settings change at all.That leaves
files.associationsdoing only what it really should, opting in to ggsql syntax highlighting for.sqlfiles.I don't think the extension should write that setting, so ideas 2 and 3 in the issue are deliberately not implemented as described. Mapping
*.sqlrewrites how every.sqlfile in every workspace is treated. It replaces the built-insource.sqlgrammar, which might arguably be the better grammar for plain SQL, and it breaks language-scoped features from other SQL tooling such as mssql, SQLTools, dbt, etc. I did try a version of this that actually changes the setting on a button press, and that turned out to require a reset command, an offer to un-write, and care about which config level to base the update on. I no longer think we should do this.Instead, opening a
.sqlfile shows a notice pointing at the setting, with a Show Setting button that opens it directly and a Don't show again button. The notice fires on each open rather than once, because an Info notification in an extension with buttons is not sticky and auto-hides after 10 seconds with no way for an extension to opt out, so a show-once notice is pretty easy to miss. A guard prevents a pile-up when many.sqlfiles open at once, and only "Don't show again" is persisted, so letting the toast age out unseen is not treated as an answer:I did consider just claiming
.sqlincontributes.languagesbut I decided that's a bad idea. Extension-contributed associations resolve last-registered-wins, which is not a contract and is fragile against other SQL extensions, whereas user-configured associations sit in the highest precedence tier and win deterministically.New
ggsql.enableSqlFilessetting (defaulttrue) can turn the.sqlbehaviour off for anyone who uses.sqlfiles with other database tooling.Also fixed: dead run actions in plain VS Code
I noticed I was making this problem worse, although it predates my work here. On
maintheeditor/title/runbuttons were already contributed withwhen: resourceLangId == ggsql && !isInDiffEditor, with no Positron gate. Every command that executes code is registered afteractivate()returns early when the Positron API is absent, so plain VS Code has always shown a run button on.gsqlfiles with no handler behind it.Everything execution-related now gates on Positron's built-in
isPositroncontext key, per the extension development docs: the twoeditor/title/runbuttons, the three keybindings, and the five run commands, which are hidden from the Command Palette through a newcommandPalettemenu entry.ggsql.createNewFileandggsql.resetSqlAssociationPromptare deliberately left ungated, since they work without Positron.ggsql-vscode/CLAUDE.mdclaimed "Outside Positron, the same commands fall back to writing query output to the active terminal." No such fallback exists anywhere insrc/, which is plausibly why this went unnoticed. That line is replaced with a description of the actual behaviour and of which side of the early return new commands belong on.A few details
languageId === 'ggsql'checks now route throughisGgsqlDocument()in the newsrc/languages.ts, rather than each growing its own|| 'sql'branch.onDidChangeCodeLenseswhen the setting changes, so toggling it takes effect without re-registering.ggsql-vscode/CLAUDE.mdlisted a non-existenticon.pngin its layout tree, which is presumably how the broken manifest path arose. Corrected..sqlfiles" section, with an explicit{#sql-files}anchor and then the setting description links to it.Verification
tsc --noEmit,eslint, and a productionesbuildrun are clean. The icon was rendered through headless Chromium at true slot geometry (16pxbackground-sizein a 16x22 box) on#ffffff,#1f1f1fand#252526, alongside the real Seti python, R and SQL glyphs and thepositron-rreference icon, to confirm legibility and matching weight.