Remove `+` buttons from diff paste data.

It would be great if when you copy and paste the diffs you wouldn't see the pluses in there. 99% of the time I am trying to copy code to test it somewhere.

Keep the + and - and remove the line numbers from the copy paste data.

I suggest using a data-line-number attribute for the number and in the css you can use :before with content: attr(data-line-number);. This will make the line number unselectable. Via this awesome blog post https://davidwalsh.name/css-content-attr

Added frontend label

Reassigned to @jschatz1

Milestone changed to 8.6

Thank you! It can be very annoying. A work around is to view raw file.

mentioned in merge request !3154 (closed)

Assignee removed

We will keep the + from the diff and remove the line numbers. Sometimes you do want plus buttons in diffs.

Reassigned to @elvongray

Milestone changed to 8.7

Assignee removed

Reassigned to @annabeldunstone

If you go the CSS and data- attribute route, make sure this doesn't break the ability to click the line numbers to get an anchor link. See https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/line_highlighter.js.coffee

cc'ing @DouweM, just to antagonize him.

@rspeicher I hate you

Everyone else, I love you.

Anyway. My approach was to directly edit the copied content using the new HTML5 JS clipboard APIs, to only get the content from the copied .line_content nodes, not the gutter. I think that makes more sense than moving the line numbers to data attributes.

We're gonna use the same approach of modifying the clipboard to allow copying a rendered comment and pasting it as Markdown.

@rspeicher you should still have my JS for removing the line numbers ;) I can't get at it right now.

@rspeicher Do you know the branch name of @DouweM 's changes?

@DouweM @rspeicher if this can be done using only CSS and HTML I think we should go for that route. Much simpler then modifying the users clipboard data.

Do you know the branch name of @DouweM 's changes?

@annabeldunstone They were just snippets in our chat DMs. Useless!

Here's the one for diffs:

$(document).on('copy', ".diff-content", function(e) {
  if (!e.originalEvent.clipboardData) return;

  var copiedDocument = $(window.getSelection().getRangeAt(0).cloneContents());
  var lines = copiedDocument.find(".line_content");
  if (lines.length) {
    e.preventDefault();

    var text = copiedDocument.find(".line_content").text();
    e.originalEvent.clipboardData.setData('Text', text);
    e.originalEvent.clipboardData.setData('x-gitlab-is-diff', true);
  }
});

$(document).on('paste', ".markdown-area", function(e) {
  if (!e.originalEvent.clipboardData) return;
  if (!e.originalEvent.clipboardData.getData('x-gitlab-is-diff')) return;

  var text = e.originalEvent.clipboardData.getData('Text');

  e.preventDefault();

  document.execCommand('insertText', false, "```diff\n" + text + "\n```");
});

@jschatz1 I'm worried that using only CSS/HTML will break the linking functionality. But if you can get that working with CSS-based line numbers I'm all for!

@DouweM Don't worry we'll get it working.

If it's even remotely possible to do something with only CSS and HTML I think we should always try that first.

@jschatz1 Hah, forgive me any unintended offense :) I know, I was simply suggesting an alternative method that would not mess with the HTML. I agree we should try that first!

@jschatz1 There's no CSS attribute to disable copy/paste is there?

I'm just messing with you. There is CSs to disable copy and paste.

I've got the copy/paste removing the line numbers and plus/minuses by using CSS & removing some of the HTML. Still in progress though!

@annabeldunstone awesome

@jschatz1 still getting the hang of what is and isn't joking with you :)

@DouweM. I'm always joking.

I made this for you:

Here are 2 ways of disabling copy from css and one with minimal inline js http://codepen.io/jschatz1/pen/EKorKb

@annabeldunstone Nice!

@DouweM @rspeicher BTW The Clipboard API is not fully supported yet and the code you posted won't work in Firefox. Surprisingly it will work in Safari.

mentioned in merge request !3687 (merged)

Status changed to closed by merge request !3687 (merged)

mentioned in commit ba18e0b2

Mentioned in merge request !7133 (merged)

Mentioned in issue #23899 (closed)

Mentioned in commit 35aca1db