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

Easier copy-pasting from diffs by making +/- signs unselectable #282

Merged
merged 2 commits into from
Oct 18, 2016
Merged

Easier copy-pasting from diffs by making +/- signs unselectable #282

merged 2 commits into from
Oct 18, 2016

Conversation

jacobbearden
Copy link
Contributor

So, I had deleted my fork when cleaning up some extraneous repositories I had, so this is all of the changes from, and requested in #254.

This will close #280 if/when it is merged.

position: absolute;
font-family: Consolas, "Liberation Mono", Menlo, Courier, monospace !important;
font-size: 12px;
text-indent: -3px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things have changed since last time, make it:

    font-size: 11px;
    text-indent: -8px;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a top: 1px; too. At least on my machine.

@sindresorhus
Copy link
Member

There's still a weird problem. Try copy-pasting multiple lines of code from a diff and you'll see that all lines except the first have an extra space at the start of the line.

Example:

td.blob-code.blob-code-deletion:before {
    content: '-';
 }

Notice how the closing brace is indented with one space.

@jacobbearden
Copy link
Contributor Author

@sindresorhus
I've been looking into the problem you mentioned, and I am having trouble finding a solution.

From what I've looked at it seems to be a problem with how Chrome handles copying text from inline elements, but I'm not sure how to go about counteracting it.

@sindresorhus
Copy link
Member

I don't really know either.

// @DrewML Any idea?

@leoj3n
Copy link

leoj3n commented Aug 13, 2016

Maybe this is related?

When I copy the code pictured below (using @jacobbearden's PR):

image

I get this on my clipboard:

    font-weight: normal;
    margin-left: 4px;
 }
 /* +/- pseudo elements on diffs */
td.blob-code.blob-code-addition:before,
td.blob-code.blob-code-deletion:before {
    display: inline-block;
    position: absolute;
    top: 1px;
    font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace !important;
    font-size: 11px;
    text-indent: -8px;
    color: rgba(0, 0, 0, 0.3);
}
 td.blob-code.blob-code-addition:before {
    content: '+';
}
 td.blob-code.blob-code-deletion:before {
    content: '-';
}

Looks like code outside of (in this case, before) the + diffs has extra space indents that need to be accounted for, and the empty newlines between code blocks are lost. :/

Here is the result with the extension disabled:

    font-weight: normal;
    margin-left: 4px;
  }
 +
 +/* +/- pseudo elements on diffs */
 +td.blob-code.blob-code-addition:before,
 +td.blob-code.blob-code-deletion:before {
 +  display: inline-block;
 +  position: absolute;
 +  top: 1px;
 +  font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace !important;
 +  font-size: 11px;
 +  text-indent: -8px;
 +  color: rgba(0, 0, 0, 0.3);
 +}
 +
 +td.blob-code.blob-code-addition:before {
 +  content: '+';
 +}
 +
 +td.blob-code.blob-code-deletion:before {
 +  content: '-';
 +}

@DrewML
Copy link
Contributor

DrewML commented Aug 14, 2016

There's still a weird problem. Try copy-pasting multiple lines of code from a diff and you'll see that all lines except the first have an extra space at the start of the line.

Am I the only person seeing that weird copy/paste behavior even with the extension disabled? It doesn't seem to be this PR that's causing it for me.

1 thing to note (and it sounds crazy, but I've gone through it like 10 times) - When I log out of my GitHub account, the issue goes away. When I log back in, it comes back.

If I visit this diff with refined-github enabled, and copy code from the diff, it works exactly as expected, with no indentation issues.

Edit: This is definitely a GitHub bug. Run $(".add-line-comment").remove() in the console, and then do a copy/paste 😄

@DrewML
Copy link
Contributor

DrewML commented Aug 14, 2016

Adding this to content.css seems to fix the issue for me:

.blob-code:not(.is-hovered) .add-line-comment {
    display: none !important;
}

Someone else want to give it a try locally?

@jacobbearden
Copy link
Contributor Author

jacobbearden commented Aug 14, 2016

@DrewML
Works for me too!

Seems like the issue of blank lines being removed persists though, and a space is being added in place of the \n (before the second td down below):

Copied:
image

Result:

td.blob-code.blob-code-addition:before {
    content: '+';
}
 td.blob-code.blob-code-deletion:before {
    content: '-';
}

@DrewML
Copy link
Contributor

DrewML commented Sep 18, 2016

What do we want to do with this PR? In my opinion, the issue with collapsing empty spaces is still better than what we have without this today (copying all the +/-). Thoughts @sindresorhus @paulmolluzzo @hkdobrev?

@sindresorhus
Copy link
Member

Sure. Let's merge it. We can open an issue about improving it in the future if we find a way.

@jacobbearden Could you fix the merge conflict?

@jacobbearden
Copy link
Contributor Author

@sindresorhus Fixed 😄

@@ -513,4 +513,22 @@ hidden content area created to increase hover target

.btn-mark-unread {
margin-top: 8px;
/* +/- pseudo elements on diffs */
Copy link
Member

@sindresorhus sindresorhus Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you had a merge conflict resolve problem here.

@sindresorhus sindresorhus changed the title Remove diff signs Easier copy-pasting from diffs by making +/- signs unselectable Oct 18, 2016
@sindresorhus sindresorhus merged commit 61edca1 into refined-github:master Oct 18, 2016
@jacobbearden jacobbearden deleted the remove-diff-signs branch October 28, 2016 03:22
@fregante fregante mentioned this pull request Oct 5, 2017
@muvaf
Copy link

muvaf commented Jan 15, 2019

This feature has been removed from Refined Github since it's implemented by Github itself as announced here

However, there are users like myself who use Github Enterprise, which is not updated very frequently. It'd be awesome to have the ability to enable that feature on the preferences page. Neither GHE I'm using supports it nor Refined Github :(

@fregante
Copy link
Member

If you're on an older GHE version, it's easier to just use an older RGH. It's too much trouble to support all GHE versions that most of us can't test.

@muvaf
Copy link

muvaf commented Jan 15, 2019

Is there a documentation on how to use an older version of RGH? Couldn't find an old enough version in Firefox addons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Remove diff signs when copy-pasting code from a diff
6 participants