search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.18k stars 29.29k forks source link

Markdown: Rename Provider behaves unexpectedly (regression) #151190

Closed Jason3S closed 2 years ago

Jason3S commented 2 years ago

Issue Type: Bug

  1. Install Code Spell Checker - Visual Studio Marketplace

  2. Open a Markdown file.

  3. Edit / add a header ## Supported Languadges:

    image

  4. Fix it using the code action:

    image

  5. Results in the entire Header being changed.

    image

The issue is due to changes in the VS Code Rename Provider.

Workaround:

settings.json

"[markdown]": {
    "cSpell.fixSpellingWithRenameProvider": false
}

Note: adding language related configuration in the VS Code UI is a painful experience.

Related to: Markdown: Fixing spelling issues in Header sections changes the entire line · Issue #1987 · streetsidesoftware/vscode-spell-checker

VS Code version: Code 1.67.2 (c3511e6c69bb39013c4a4b7b9566ec1ca73fc4d5, 2022-05-17T18:20:57.384Z) OS version: Darwin x64 21.5.0 Restricted Mode: No

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz (16 x 2300)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled| |Load (avg)|3, 3, 3| |Memory (System)|32.00GB (0.13GB free)| |Process Argv|--crash-reporter-id 21ae0ece-62a2-4681-a5cf-c687ea23e753| |Screen Reader|no| |VM|0%|
Extensions (51) Extension|Author (truncated)|Version ---|---|--- terraform|4op|0.2.5 ada|Ada|23.0.6 commit-message-editor|ada|0.19.7 alignment|ann|0.3.0 vscode-zipfs|arc|3.0.0 vscode-intelephense-client|bme|1.8.2 better-toml|bun|0.3.2 scala|dal|0.0.5 vscode-jq-playground|dav|4.3.2 vscode-eslint|dba|2.2.2 gitlens|eam|12.0.7 EditorConfig|Edi|0.16.4 vscode-npm-script|eg2|0.3.25 prettier-vscode|esb|9.5.0 linter-gfortran|krv|3.0.2022042917 gitpod-desktop|git|0.0.37 vscode-graphql|Gra|0.4.6 mediawiki|jak|2.1.0 latex-workshop|Jam|8.26.0 svg|joc|1.4.18 jq-syntax-highlighting|jq-|0.0.2 language-haskell|jus|3.6.0 vscode-cfml|Kam|0.5.4 remotefs|lix|0.0.16 AWK|lug|0.0.2 Kotlin|mat|1.7.1 vscode-apache|mrm|1.2.0 vscode-puglint|mrm|2.3.0 vscode-docker|ms-|1.22.0 python|ms-|2022.6.3 vscode-pylance|ms-|2022.6.0 jupyter|ms-|2022.4.1021342353 jupyter-keymap|ms-|1.0.0 jupyter-renderers|ms-|1.0.8 remote-containers|ms-|0.234.0 remote-ssh|ms-|0.80.0 azure-account|ms-|0.10.1 cpptools|ms-|1.9.8 live-server|ms-|0.2.12 vetur|oct|0.35.0 marko|pca|0.4.0 java|red|1.7.0 vscode-yaml|red|1.7.0 mdx|sil|0.1.0 es6-mocha-snippets|spo|0.2.2 avro|str|0.5.0 code-spell-checker|str|2.2.0 code-spell-checker-portuguese-brazilian|str|2.0.6 hunspell|str|1.0.0 vscodeintellicode|Vis|1.2.21 vscode-java-debug|vsc|0.41.0
A/B Experiments ``` vsliv368:30146709 vsreu685:30147344 python383cf:30185419 vspor879:30202332 vspor708:30202333 vspor363:30204092 pythonvspyl392:30443607 pythontb:30283811 pythonvspyt551cf:30345471 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 vscoreces:30445986 pythondataviewer:30285071 vscod805:30301674 pythonvspyt200:30340761 binariesv615:30325510 bridge0708:30335490 bridge0723:30353136 vsaa593:30376534 vsc1dst:30438360 pythonvs932:30410667 wslgetstarted:30449410 pythonvsnew555:30457759 vscscmwlcmt:30465135 cppdebug:30492333 vscaac:30438847 vsclangdc:30486549 ```
vscodenpa commented 2 years ago

This issue is caused by an extension, please file it with the repository (or contact) the extension has linked in its overview in VS Code or the marketplace for VS Code. See also our issue reporting guidelines.

Happy Coding!

mjbvz commented 2 years ago

See https://code.visualstudio.com/updates/v1_67#_markdown-rename-headers

Jason3S commented 2 years ago

@mjbvz,

Thank you for the quick reply.

So this is a bug in the Markdown-rename-headers, how do I file a bug against that?

The spell checker is doing the right thing, by using the rename provider. But because only the misspelled word is selected, the markdown rename provider is renaming the whole thing.

Jason3S commented 2 years ago

@mjbvz,

To be clear, the other rename providers do the right thing.

For example, if there is a misspelling in a typescript variable, like: const socketRecieverID = 22, the spell checker only changes Reciever into Receiver and the typescript rename provider does the right thing and changes all the instances correctly to socketReceiverID. I would expect the markdown rename provider to do the same thing.

Jason3S commented 2 years ago

I took a look at the spell checker code: https://github.com/streetsidesoftware/vscode-spell-checker/blob/6781e5e952a0007f78ceff1d157763709c2c7762/packages/client/src/commands.ts#L212-L236

There is no good way for an extension to do the right thing without knowing a bit of the magic behind the markdown rename provider.

Do you have a suggestion on a clean multi-language way to support this?

Jason3S commented 2 years ago

@mjbvz,

I took a look at the Markdown Rename Provider trying to find a universal way to fix a spelling error in a header or other location. This does not seem to be possible due to internal logic and assumptions in the Markdown Rename Provider.

The Markdown Rename Provider does not take into account the Position passed via the rename command other than to use it as a way to find the reference. It then proceeds to use a hidden value ref.headerTextLocation without compensating for the actual Position of the new text.

https://github.com/microsoft/vscode/blob/711c34bdb6c4da154d3c717956a0c0c51bae8de4/extensions/markdown-language-features/src/languageFeatures/rename.ts#L203-L219

Jason3S commented 2 years ago

@mjbvz,

What is the harm in having the reference provider return ref.headerTextLocation instead of ref.location when it is a MdHeaderReference?

https://github.com/microsoft/vscode/blob/711c34bdb6c4da154d3c717956a0c0c51bae8de4/extensions/markdown-language-features/src/languageFeatures/references.ts#L77-L83

Is anyone using the fact that the Reference Provider returns the # as part of the reference? Isn't the # a bit like const in TypeScript?

mjbvz commented 2 years ago

@Jason3S You can try making the change but to get it accepted you need to show that it doesn't cause any issues with the various find all reference UIs