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

Should markdown url smart paste be disabled if the user hasn't made a selection? #188871

Closed
Tracked by #189320
mjbvz opened this issue Jul 25, 2023 · 9 comments
Closed
Tracked by #189320
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug markdown Markdown support issues verified Verification succeeded

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 25, 2023

  1. Copy http://example.com
  2. Paste into a md file without any making any selection

Bug
Currently this inserts a markdown link. However this is different from what GitHub does. Github seems to only create a link if you have a selection

We should at least think about:

  • Matching GitHub's behavior
  • Adding a setting that lets us match GitHub's behavior
@mjbvz mjbvz changed the title Should markdown url smart paste be disabled if the user hasn't made a selection Should markdown url smart paste be disabled if the user hasn't made a selection? Jul 25, 2023
@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 25, 2023

Last iteration I think @isidorn had another related idea: disabling smart paste if you are inside of a word #186289

@isidorn
Copy link
Contributor

isidorn commented Jul 26, 2023

I agree we should think about selection + smart paste. It boils down to - how often do users want to have a URL in markdown as a full https link? If the answer is > 30% then I think we should only have smart paste on selection.
To get a sense for this question - we could analyse top markdown repos on GitHub and see how often is a http link in Markdown without it being rendered as a markdown link.

As for disabling smart paste if in a word - I still think this makes sense. I like that we have disable smart paste if in a link - thanks for that!

@af4jm
Copy link

af4jm commented Aug 6, 2023

I'm seeing that same behavior, even though I have "markdown.editor.pasteUrlAsFormattedLink.enabled": "never" in my settings... which is obviously getting ignored... if I paste a link at [test](|) (where | marks the cursor position), I also get the smart link Sorry, found it set to true in the folder settings that was overriding the Profile setting, after changing that I'm seeing the feature behave as documented

@MeghanKulkarni MeghanKulkarni removed this from the August 2023 milestone Aug 16, 2023
@lgarron
Copy link

lgarron commented Sep 21, 2023

This also keeps tripping me up. I assumed I had chosen a bad configuration, but it sounds like this actually can't be configured easily. Both "always" and "smart" will try to create linked text, making it annoying to paste a URL directly.

I also agree that GitHub behaviour is much more practical, and it would also be nice for the behaviour match for those of us who spend most of our programming time in VS Code and GitHub.

If anyone has a combination of shortcuts and settings that matches GitHub, I'd love to know. Else, I'd love to learn a way to set ⇧⌘V to paste the link without formatting.

@lgarron
Copy link

lgarron commented Sep 21, 2023

Else, I'd love to learn a way to set ⇧⌘V to paste the link without formatting.

Okay, I figured out how to do this:

// keybindings.json
[
  // Disable existing ⇧⌘V shortcuts — modify to taste
  {
    "key": "shift+cmd+v",
    "command": "-notebook.cell.pasteAbove",
    "when": "notebookEditorFocused && !inputFocus"
  },
  {
    "key": "shift+cmd+v",
    "command": "-markdown.showPreview",
    "when": "!notebookEditorFocused && editorLangId == 'markdown'"
  },
  // Enable ⌥⌘V for plain text paste
  {
    "key": "shift+cmd+v",
    "command": "editor.action.pasteAs",
    "args": {
      "id": "text"
    },
    "when": "editorLangId == 'markdown'"
  }
]

In theory it should be possible to use editorHasSelection to match GitHub's behaviour, but the following doesn't seem to work:

[
  {
    "key": "shift+v",
    "command": "editor.action.pasteAs",
    "args": {
      "id": "text"
    },
    "when": "!editorHasSelection && editorLangId == 'markdown'"
  }
]

The next approach would be to trigger "smart" paste only when editorHasSelection, but I haven't figured out a way to do that yet.

@lgarron
Copy link

lgarron commented Sep 21, 2023

The next approach would be to trigger "smart" paste only when editorHasSelection, but I haven't figured out a way to do that yet.

Okay, I think I've got it:

// keybindings.json
[
  {
    "key": "cmd+v",
    "command": "editor.action.pasteAs",
    "args": {
      "id": "text"
    },
    "when": "!editorHasSelection && editorLangId == 'markdown'"
  },
  {
    "key": "cmd+v",
    "command": "editor.action.clipboardPasteAction",
    "args": {},
    "when": "editorHasSelection || editorLangId != 'markdown'"
  },
  {
    "key": "cmd+v",
    "command": "-editor.action.clipboardPasteAction"
  }
]

This also seems to work, but I'm not 100% confident about the priority of conflicting conditions:

// keybindings.json
[
  {
    "key": "cmd+v",
    "command": "editor.action.pasteAs",
    "args": {
      "id": "text"
    },
    "when": "!editorHasSelection && editorLangId == 'markdown'"
  },
  {
    "key": "cmd+v",
    "command": "-editor.action.clipboardPasteAction",
    "when": "!editorHasSelection && editorLangId == 'markdown'"
  }
]

But this took over half an hour of experimentation + digging through the VS Code source code and documentation. It also doesn't take into account:

  • notebookEditorFocused
  • inputFocus / editorTextFocus
  • ⇧⌘V to swap the default replacement behaviour with the opposite.

I presume changing the default behaviour would involve adding an early return here, when there is currently no selection:

@lgarron
Copy link

lgarron commented Sep 21, 2023

  • notebookEditorFocused
  • inputFocus / editorTextFocus
  • ⇧⌘V to swap the default replacement behaviour with the opposite.

Here's a try for a version that takes these into account:

// keybindings.json
[
  {
    "key": "cmd+v",
    "command": "editor.action.pasteAs",
    "args": {
      "id": "text"
    },
    "when": "inputFocus && !editorHasSelection && editorLangId == 'markdown'"
  },
  {
    "key": "cmd+v",
    "command": "-editor.action.clipboardPasteAction",
    "when": "inputFocus && !editorHasSelection && editorLangId == 'markdown'"
  },
  {
    "key": "shift-cmd+v",
    "command": "editor.action.pasteAs",
    "args": {
      "id": "text"
    },
    "when": "inputFocus && editorHasSelection"
  }
]

It avoids clobbering the ⇧⌘V shortcut for notebook.cell.pasteAbove but still has the following issues:

  • Incorrect text selection after paste.
  • Hardcoded to macOS.

lgarron added a commit to lgarron/dotfiles that referenced this issue Sep 21, 2023
@Alecton4
Copy link

Can we rename the current smart option to something like selectionOnly and restore the behavior of the previous smart option? According to this comment, smart now only creates a markdown link if some text is selected beforehand, which I guess is not "smart" enough.

I'm using Markdown to write notes, and reference links to other websites are often used. With the current smart option, I have to 1) type/paste the site title, 2) select the title, and 3) paste URL. But with the previous smart behavior, it's 1) paste URL, 2) press Tab, and 3) type/paste the site title. Selecting is more painful than just pressing Tab especially when lots of links need to be inserted.

@mjbvz mjbvz added bug Issue identified by VS Code Team member as probable bug markdown Markdown support issues labels Dec 5, 2023
@mjbvz mjbvz added this to the Backlog milestone Dec 5, 2023
@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 10, 2024

This is now the default. You can then use the paste widget to switch after the paste has taken place

@Alecton4 Good idea. I think I'll add a new option called smartWithSelection that replaces smart and is the default. Then smart will be a new option that always applies, even for empty selections (names may change)

@mjbvz mjbvz closed this as completed Jan 10, 2024
@mjbvz mjbvz modified the milestones: Backlog, December / January 2024 Jan 10, 2024
@mjbvz mjbvz mentioned this issue Jan 23, 2024
3 tasks
@aiday-mar aiday-mar added the verified Verification succeeded label Jan 25, 2024
@aiday-mar aiday-mar added this to the December / January 2024 milestone Feb 6, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug markdown Markdown support issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants