List for cgit developers and users
 help / color / mirror / Atom feed
From: john at keeping.me.uk (John Keeping)
Subject: [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
Date: Sun, 24 Jun 2018 14:39:35 +0100	[thread overview]
Message-ID: <20180624133935.GN6584@john.keeping.me.uk> (raw)
In-Reply-To: <26302F2E-37BA-4457-8713-23B6372B8DBE@warmcat.com>

On Sun, Jun 24, 2018 at 08:00:08PM +0800, Andy Green wrote:
> 
> 
> On June 24, 2018 7:42:33 PM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
> >On Sun, Jun 24, 2018 at 10:44:54AM +0800, Andy Green wrote:
> >> Since the only reason to click on the line number links
> >> is to get the corresponding #URL to share, this patch
> >> makes that process more convenient by copying the
> >> highlit area, be it a single line or a range, to the
> >> clipboard on each click of the line number links.
> >
> >As a user, I'd find this surprising and probably quite annoying.
> >
> >I strongly prefer that software not overwrite the clipboard contents
> >without an explicit request to do so.  A quick survey suggests none of
> >GitHub, Gitlab or repo.or.cz (GitWeb) behave in this way.
> 
> Is there another possible intention behind clicking on the line number
> links I am missing?

One I can think of (given your recent patches) is to highlight a line
when pointing it out to someone else looking at the same display.  I
have definitely used this with other web interfaces in the past.

> If not, literally the only reason to click on them is because you
> intend to copy the #URL to the clipboard.
> 
> Then that is "an explicit request", isn't it?

In the X11 world there are separate selection buffers for selecting text
and for Ctrl-C/V behaviour [1].  It might not be unreasonable to set
PRIMARY to the URL, but setting CLIPBOARD is clearly wrong.

My use of "explicit" above is consistent with the use in this spec,
unless the user has selected Copy, or pressed Ctrl-C there is no
expectation that the clipboard will be overwritten.

Certainly for an X11 user overwriting CLIPBOARD when clicking a
hyperlink fails the principle of least surprise, and I think that
applies to all users because this isn't how web pages work: on any other
page, if I want to copy a link I right-click and copy the link.  I don't
expect the clipboard to be overwritten just because I clicked a
hyperlink.

[1] https://specifications.freedesktop.org/clipboards-spec/clipboards-0.1.txt

> >> Signed-off-by: Andy Green <andy at warmcat.com>
> >> ---
> >>  cgit.js |   36 ++++++++++++++++++++++++++++++++----
> >>  1 file changed, 32 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/cgit.js b/cgit.js
> >> index 2cfad29..e2c3799 100644
> >> --- a/cgit.js
> >> +++ b/cgit.js
> >> @@ -89,8 +89,29 @@ function cgit_line_range_highlight()
> >>  		e.scrollIntoView(true);
> >>  }
> >>  
> >> +function cgit_copy_clipboard(value)
> >> +{
> >> +	var inp = document.createElement("textarea");
> >> +	var e = document.getElementById("linenumber_td");
> >> +
> >> +	inp.type = "text";
> >> +	inp.value = value;
> >> +	/* hidden style stops it working for clipboard */
> >> +	inp.setAttribute('readonly', '');
> >> +	inp.style.position = "absolute";
> >> +	inp.style.left = "-1000px";
> >> +
> >> +	e.appendChild(inp);
> >> +
> >> +	inp.select();
> >> +
> >> +	document.execCommand("copy");
> >> +
> >> +	inp.remove();
> >> +}
> >> +
> >>  function cgit_line_range_click(e) {
> >> -	var t = e.target.id;
> >> +	var t = e.target.id, cp;
> >>  
> >>  	cgit_line_range_override = null;
> >>  
> >> @@ -101,13 +122,13 @@ function cgit_line_range_click(e) {
> >>  	 * is called, and override it there.
> >>  	 */
> >>  
> >> -	if (window.location.hash && window.location.hash.indexOf("-") < 0)
> >> +	if (window.location.hash && window.location.hash.indexOf("-") < 0)
> >{
> >>  		if (parseInt(window.location.hash.substring(2)) <
> >>  		    parseInt(t.substring(1))) /* forwards */
> >> -			cgit_line_range_override =
> >> +			cp = cgit_line_range_override =
> >>  				window.location + '-' + t.substring(1);
> >>  		else /* backwards */
> >> -			cgit_line_range_override =
> >> +			cp = cgit_line_range_override =
> >>  				window.location.href.substring(0,
> >>  					window.location.href.length -
> >>  					window.location.hash.length) +
> >> @@ -115,6 +136,13 @@ function cgit_line_range_click(e) {
> >>  					window.location.href.substring(
> >>  						window.location.href.length -
> >>  						window.location.hash.length + 2);
> >> +	} else
> >> +		cp = window.location.href.substring(0,
> >> +                                        window.location.href.length
> >-
> >> +                                        window.location.hash.length)
> >+
> >> +			'#n' + t.substring(1);
> >> +
> >> +	cgit_copy_clipboard(cp);
> >>  }
> >>  
> >>  /* line range highlight */
> >> 
> >> _______________________________________________
> >> CGit mailing list
> >> CGit at lists.zx2c4.com
> >> https://lists.zx2c4.com/mailman/listinfo/cgit
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit


  reply	other threads:[~2018-06-24 13:39 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
2018-06-21  5:42 ` [PATCH 1/3] ui-shared: introduce line range highlight javascript andy
2018-06-21  7:03   ` list
2018-06-21  7:30     ` andy
2018-06-21  5:42 ` [PATCH 2/3] ui-tree: use the line range highlight script andy
2018-06-21  5:43 ` [PATCH 3/3] ui-blame: " andy
2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
2018-06-21  9:34   ` [PATCH v2 1/5] config: add js andy
2018-06-23 10:20     ` john
2018-06-23 10:34       ` andy
2018-06-21  9:34   ` [PATCH v2 2/5] cgit.js: introduce andy
2018-06-23 10:18     ` john
2018-06-21  9:34   ` [PATCH v2 3/5] ui-shared: introduce line range highlight javascript andy
2018-06-23 10:17     ` john
2018-06-24  2:37       ` andy
2018-06-21  9:35   ` [PATCH v2 4/5] ui-tree: use the line range highlight script andy
2018-06-21  9:35   ` [PATCH v2 5/5] ui-blame: " andy
2018-06-22 23:01   ` [PATCH v2 1/2] cgit.js: make line range highlight responsive to url changes andy
2018-06-22 23:02   ` [PATCH v2 2/2] cgit.js: line range highlight: improve vertical scroll logic andy
2018-06-23  7:45   ` [PATCH v2] cgit.js: line range highlight: always hook hashchange in case hash added andy
2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
2018-06-24  2:44   ` [PATCH v3 1/6] config: add js andy
2018-06-24 11:01     ` john
2018-06-24  2:44   ` [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript andy
2018-06-24 11:28     ` john
2018-06-25  2:04       ` andy
2018-06-24  2:44   ` [PATCH v3 3/6] cgit.js: line range highlight: make responsive to url changes andy
2018-06-24  2:44   ` [PATCH v3 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
2018-06-24  2:44   ` [PATCH v3 5/6] line-range-highlight: onclick handler and range selection andy
2018-06-24 11:35     ` john
2018-06-25  2:07       ` andy
2018-06-24  2:44   ` [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click andy
2018-06-24 11:42     ` john
2018-06-24 12:00       ` andy
2018-06-24 13:39         ` john [this message]
2018-06-24 15:06           ` andy
2018-06-24 16:03             ` john
2018-06-25  0:46               ` andy
2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
2018-06-25  5:49   ` [PATCH v4 1/6] config: add js andy
2018-06-26  8:03     ` list
2018-06-25  5:49   ` [PATCH v4 2/6] cgit.js: line range highlight: introduce javascript andy
2018-06-25  5:49   ` [PATCH v4 3/6] cgit.js: line range highlight: make responsive to url changes andy
2018-06-25  5:50   ` [PATCH v4 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
2018-06-25  5:50   ` [PATCH v4 5/6] line-range-highlight: onclick handler and range selection andy
2018-06-25  5:50   ` [PATCH v4 6/6] line-range-highlight: copy URL to clipboard UI andy
2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
2018-06-26 11:25   ` [PATCH v5 1/6] config: add js andy
2018-06-26 11:25   ` [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript andy
2018-06-27 18:02     ` Jason
2018-06-27 21:45       ` andy
2018-06-28 23:58         ` [PATCH] cgit.css: add copyright lines andy
2018-06-26 11:25   ` [PATCH v5 3/6] cgit.js: line range highlight: make responsive to url changes andy
2018-06-26 11:25   ` [PATCH v5 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
2018-06-26 11:25   ` [PATCH v5 5/6] line-range-highlight: onclick handler and range selection andy
2018-06-26 11:51     ` [PATCH v5-ninjaedit] " andy
2018-06-26 11:25   ` [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI andy
2018-06-27 18:07     ` Jason
2018-06-27 23:24       ` andy
2018-06-27 23:30         ` Jason
2018-06-27 23:38           ` andy
2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
2018-06-29  1:40   ` [PATCH v6 1/7] config: add js andy
2018-06-29  6:14     ` list
2018-06-29  6:16       ` [PATCH v6-ninjaedit] " andy
2018-06-29  6:31         ` [PATCH v6-ninjaedit2] " andy
2018-06-29  6:33         ` [PATCH v6-ninjaedit] " list
2018-06-29  1:40   ` [PATCH v6 2/7] cgit.js: line range highlight: introduce javascript andy
2018-06-29  1:40   ` [PATCH v6 3/7] cgit.js: line range highlight: make responsive to url changes andy
2018-06-29  1:40   ` [PATCH v6 4/7] cgit.js: line range highlight: improve vertical scroll logic andy
2018-06-29  1:40   ` [PATCH v6 5/7] line-range-highlight: onclick handler and range selection andy
2018-06-29  1:40   ` [PATCH v6 6/7] line-range-highlight: burger menu and popup menu andy
2018-06-29  1:40   ` [PATCH v6 7/7] line-range-highlight: copy text andy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180624133935.GN6584@john.keeping.me.uk \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).