List for cgit developers and users
 help / color / mirror / Atom feed
* Highlighting lines or line ranges in tree view
@ 2018-06-20  9:39 andy
  2018-06-21  5:42 ` [PATCH 1/3] ui-shared: introduce line range highlight javascript andy
                   ` (7 more replies)
  0 siblings, 8 replies; 73+ messages in thread
From: andy @ 2018-06-20  9:39 UTC (permalink / raw)


Hi -

It's convenient to be able to provide a URL that points to a specific 
line in tree view.  cgit provides links on the line numbers it produces 
to facilitate that, eg

https://libwebsockets.org/git/libwebsockets/tree/minimal-examples/ws-server/minimal-ws-server-echo/minimal-ws-server-echo.c#n43

However when you get there, there is no visual indication about the 
specific line.  If the line happens to be early enough in the file that 
the end of the file is not in view, then the line is vertically arranged 
by the browser to be the first visible line.  But if not, at least in 
firefox the vertical position is arranged so the last line of the file 
appears at the bottom and the first visible line is whatever it happens 
to be then, unrelated to the target in the URL.

I guess with some JS and planning, it would be possible to have the page 
parse the raw URL and highlight the matching lines, also ranges of lines 
from URLs like ...#n43-48

But I think today, there's no JS in cgit.  What's the feeling about this 
kind of enhancement?  Stuff will be no worse than it is now if JS is 
disabled on the client.  If JS in cgit is not verboten, what 
considerations should we think about from maintainability, security 
standpoints when integrating it?

-Andy


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/3] ui-shared: introduce line range highlight javascript
  2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
@ 2018-06-21  5:42 ` andy
  2018-06-21  7:03   ` list
  2018-06-21  5:42 ` [PATCH 2/3] ui-tree: use the line range highlight script andy
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-21  5:42 UTC (permalink / raw)


This adds a small css class and a function in ui-shared that
emits canned javascript to interpret the # part of the URL
on the client, and apply a highlight to filtered source.

Unlike blame highlight boxes which use generated divs, this
applied a computed absolute, transparent highlight over the
affected line(s) on the client side.

The # part of the URL is defined to not be passed to the server,
so the highlight can't be rendered on the server side.
However this has the advantage that the line range highlight
can operate on /blame/ urls trivially, since it doesn't
conflict with blame's generated div scheme.

pointer-events: none is used on the highlight overlay div to
allow the user to cut-and-paste in the highlit region and
click on links underneath normally.

The JS supports highlighting single lines as before like #n123
and also ranges of lines like #n123-135.

Because the browser can no longer automatically scroll to the
element in the second case, the JS also takes care of extracting
the range start element and scrolling to it dynamically.

Tested on Linux Firefox 60 + Linux Chrome 67

Examples:

https://warmcat.com/git/cgit/tree/ui-shared.c#n1146
https://warmcat.com/git/cgit/tree/ui-shared.c#n1146-1164
https://warmcat.com/git/cgit/blame/ui-shared.c#n1146-1164

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css    |    8 ++++++++
 ui-shared.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 ui-shared.h |    1 +
 3 files changed, 61 insertions(+)

diff --git a/cgit.css b/cgit.css
index 61bbd49..7cb0fd6 100644
--- a/cgit.css
+++ b/cgit.css
@@ -368,6 +368,14 @@ div#cgit table.blame td.lines > div > pre {
 	top: 0;
 }
 
+div#cgit div.line_range {
+	position: absolute;
+	pointer-events: none;
+	left: 0px;
+	z-index: 1;
+	background-color: rgba(255, 255, 0, 0.2);
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/ui-shared.c b/ui-shared.c
index 082a6f1..c8f4d8f 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1237,3 +1237,55 @@ void cgit_set_title_from_path(const char *path)
 	strcat(new_title, ctx.page.title);
 	ctx.page.title = new_title;
 }
+
+void cgit_emit_line_range_highlight_script(int parent_level)
+{
+	html("<script>\n"
+	     "function cgit_line_range_highlight()\n"
+	     "{\n"
+	     "	var h = window.location.hash, l1 = 0, l2 = 0, e, t;\n"
+	     "\n"
+	     "	l1 = parseInt(h.substring(2));\n"
+	     "	t = h.indexOf(\"-\");\n"
+	     "	if (t >= 1)\n"
+	     "		l2 = parseInt(h.substring(t + 1));\n"
+	     "	else\n"
+	     "		l2 = l1;\n"
+	     "\n"
+	     "	if (l1) {\n"
+	     "		var lh, t = 0, e1, de;\n"
+	     "\n"
+	     "		e1 = e = document.getElementById('n' + l1);\n"
+	     "		if (e) {\n"
+	     "\n"
+	     "			while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; } \n"
+	     "\n"
+	     "			de = document.createElement(\"DIV\");"
+	     "\n"
+	     "			de.className = \"line_range\";\n"
+	     "			de.style.bottom = e.style.bottom;\n"
+	     "			de.style.top = t + 'px';\n");
+	if (!parent_level)
+		html("			de.style.width = e.parentElement.parentElement.parentNode.offsetWidth + 'px';\n");
+	else
+		html("			de.style.width = e.parentElement.parentElement.parentNode.parentNode.offsetWidth + 'px';\n");
+	html("			de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';\n"
+	     "\n"
+	     "			e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);\n"
+	     "\n"
+	     "			while (l1 <= l2) {\n"
+	     "				var e1;\n"
+	     "				e1 = document.getElementById('n' + l1++);\n"
+	     "      			e1.style.backgroundColor = \"yellow\";\n"
+	     "			}\n"
+	     "\n"
+	     "			e.scrollIntoView(true);\n"
+	     "		}\n"
+	     "	}\n"
+	     "}\n"
+	     "\n"
+	     "document.addEventListener(\"DOMContentLoaded\", function() {"
+	     "	cgit_line_range_highlight();\n"
+	     "}, false);\n"
+	     "</script>\n");
+}
diff --git a/ui-shared.h b/ui-shared.h
index e78b5fd..8b0cddc 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -89,4 +89,5 @@ extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
 				       const char *page);
 
 extern void cgit_set_title_from_path(const char *path);
+extern void cgit_emit_line_range_highlight_script(int parent_level);
 #endif /* UI_SHARED_H */



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 2/3] ui-tree: use the line range highlight script
  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  5:42 ` andy
  2018-06-21  5:43 ` [PATCH 3/3] ui-blame: " andy
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-21  5:42 UTC (permalink / raw)


This allows it to work in /tree/ and /source/ URLs.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 ui-tree.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..b66cabe 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -226,6 +226,8 @@ static void print_object(const struct object_id *oid, char *path, const char *ba
 	} else
 		print_buffer(basename, buf, size);
  
+	cgit_emit_line_range_highlight_script(0);
+
 	free(buf);
 	free(mimetype);
 }



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 3/3] ui-blame: use the line range highlight script
  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  5:42 ` [PATCH 2/3] ui-tree: use the line range highlight script andy
@ 2018-06-21  5:43 ` andy
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-21  5:43 UTC (permalink / raw)


This allows it to work on /blame/ URLs.

Blame has an extra layer of parent elements which
has to be taken into account by the JS when finding
the highlight width, hence the 1 arg to the emit
function.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 ui-blame.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..ef8a41b 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -212,6 +212,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	html("</tr>\n</table>\n");
 
+	cgit_emit_line_range_highlight_script(1);
 	cgit_print_layout_end();
 
 cleanup:



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/3] ui-shared: introduce line range highlight javascript
  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
  0 siblings, 1 reply; 73+ messages in thread
From: list @ 2018-06-21  7:03 UTC (permalink / raw)


Andy Green <andy at warmcat.com> on Thu, 2018/06/21 13:42:
> This adds a small css class and a function in ui-shared that
> emits canned javascript to interpret the # part of the URL
> on the client, and apply a highlight to filtered source.
> 
> Unlike blame highlight boxes which use generated divs, this
> applied a computed absolute, transparent highlight over the
> affected line(s) on the client side.
> 
> The # part of the URL is defined to not be passed to the server,
> so the highlight can't be rendered on the server side.
> However this has the advantage that the line range highlight
> can operate on /blame/ urls trivially, since it doesn't
> conflict with blame's generated div scheme.
> 
> pointer-events: none is used on the highlight overlay div to
> allow the user to cut-and-paste in the highlit region and
> click on links underneath normally.
> 
> The JS supports highlighting single lines as before like #n123
> and also ranges of lines like #n123-135.
> 
> Because the browser can no longer automatically scroll to the
> element in the second case, the JS also takes care of extracting
> the range start element and scrolling to it dynamically.
> 
> Tested on Linux Firefox 60 + Linux Chrome 67
> 
> Examples:
> 
> https://warmcat.com/git/cgit/tree/ui-shared.c#n1146
> https://warmcat.com/git/cgit/tree/ui-shared.c#n1146-1164
> https://warmcat.com/git/cgit/blame/ui-shared.c#n1146-1164
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> ---
>  cgit.css    |    8 ++++++++
>  ui-shared.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ui-shared.h |    1 +
>  3 files changed, 61 insertions(+)
> 
> diff --git a/cgit.css b/cgit.css
> index 61bbd49..7cb0fd6 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -368,6 +368,14 @@ div#cgit table.blame td.lines > div > pre {
>  	top: 0;
>  }
>  
> +div#cgit div.line_range {
> +	position: absolute;
> +	pointer-events: none;
> +	left: 0px;
> +	z-index: 1;
> +	background-color: rgba(255, 255, 0, 0.2);
> +}
> +
>  div#cgit table.bin-blob {
>  	margin-top: 0.5em;
>  	border: solid 1px black;
> diff --git a/ui-shared.c b/ui-shared.c
> index 082a6f1..c8f4d8f 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -1237,3 +1237,55 @@ void cgit_set_title_from_path(const char *path)
>  	strcat(new_title, ctx.page.title);
>  	ctx.page.title = new_title;
>  }
> +
> +void cgit_emit_line_range_highlight_script(int parent_level)
> +{
> +	html("<script>\n"
> +	     "function cgit_line_range_highlight()\n"
> +	     "{\n"
> +	     "	var h = window.location.hash, l1 = 0, l2 = 0, e,
> t;\n"
> +	     "\n"
> +	     "	l1 = parseInt(h.substring(2));\n"
> +	     "	t = h.indexOf(\"-\");\n"
> +	     "	if (t >= 1)\n"
> +	     "		l2 = parseInt(h.substring(t + 1));\n"
> +	     "	else\n"
> +	     "		l2 = l1;\n"
> +	     "\n"
> +	     "	if (l1) {\n"
> +	     "		var lh, t = 0, e1, de;\n"
> +	     "\n"
> +	     "		e1 = e = document.getElementById('n' +
> l1);\n"
> +	     "		if (e) {\n"
> +	     "\n"
> +	     "			while (e1) { if (e1.offsetTop) t +=
> e1.offsetTop; e1 = e1.offsetParent; } \n"
> +	     "\n"
> +	     "			de =
> document.createElement(\"DIV\");"
> +	     "\n"
> +	     "			de.className = \"line_range\";\n"
> +	     "			de.style.bottom = e.style.bottom;\n"
> +	     "			de.style.top = t + 'px';\n");
> +	if (!parent_level)
> +		html("			de.style.width =
> e.parentElement.parentElement.parentNode.offsetWidth + 'px';\n");
> +	else
> +		html("			de.style.width =
> e.parentElement.parentElement.parentNode.parentNode.offsetWidth + 'px';\n");
> +	html("			de.style.height = ((l2 - l1 + 1) *
> e.offsetHeight) + 'px';\n"
> +	     "\n"
> +	     "
> e.parentElement.parentElement.insertBefore(de,
> e.parentElement.parentElement.firstChild);\n"
> +	     "\n"
> +	     "			while (l1 <= l2) {\n"
> +	     "				var e1;\n"
> +	     "				e1 =
> document.getElementById('n' + l1++);\n"
> +	     "      			e1.style.backgroundColor =
> \"yellow\";\n"
> +	     "			}\n"
> +	     "\n"
> +	     "			e.scrollIntoView(true);\n"
> +	     "		}\n"
> +	     "	}\n"
> +	     "}\n"
> +	     "\n"
> +	     "document.addEventListener(\"DOMContentLoaded\", function() {"
> +	     "	cgit_line_range_highlight();\n"
> +	     "}, false);\n"
> +	     "</script>\n");
> +}

I would prefer to have this in a file cgit.js (much like cgit.css).

> diff --git a/ui-shared.h b/ui-shared.h
> index e78b5fd..8b0cddc 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -89,4 +89,5 @@ extern void cgit_add_hidden_formfields(int incl_head, int
> incl_search, const char *page);
>  
>  extern void cgit_set_title_from_path(const char *path);
> +extern void cgit_emit_line_range_highlight_script(int parent_level);
>  #endif /* UI_SHARED_H */
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit



-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20180621/38216df4/attachment.asc>


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/3] ui-shared: introduce line range highlight javascript
  2018-06-21  7:03   ` list
@ 2018-06-21  7:30     ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-21  7:30 UTC (permalink / raw)




On 06/21/2018 03:03 PM, Christian Hesse wrote:
> Andy Green <andy at warmcat.com> on Thu, 2018/06/21 13:42:
>> This adds a small css class and a function in ui-shared that
>> emits canned javascript to interpret the # part of the URL
>> on the client, and apply a highlight to filtered source.

      "      			e1.style.backgroundColor =
>> \"yellow\";\n"
>> +	     "			}\n"
>> +	     "\n"
>> +	     "			e.scrollIntoView(true);\n"
>> +	     "		}\n"
>> +	     "	}\n"
>> +	     "}\n"
>> +	     "\n"
>> +	     "document.addEventListener(\"DOMContentLoaded\", function() {"
>> +	     "	cgit_line_range_highlight();\n"
>> +	     "}, false);\n"
>> +	     "</script>\n");
>> +}
> 
> I would prefer to have this in a file cgit.js (much like cgit.css).

OK... for the main part of it, it makes sense.  And have it brought in 
in HEAD.  Because I think the direction of the JS stuff is that it will 
only want to increase once it's a thing.

For the stuff using it or not though, it will still want to have its own 
little <script> and schedule or call the main stuff inline.

-Andy


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 0/5] line range highlight
  2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
                   ` (2 preceding siblings ...)
  2018-06-21  5:43 ` [PATCH 3/3] ui-blame: " andy
@ 2018-06-21  9:34 ` andy
  2018-06-21  9:34   ` [PATCH v2 1/5] config: add js andy
                     ` (7 more replies)
  2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
                   ` (3 subsequent siblings)
  7 siblings, 8 replies; 73+ messages in thread
From: andy @ 2018-06-21  9:34 UTC (permalink / raw)


The following series adds the ability to direct the browser
to highlight specific lines and ranges of lines in
/tree/, /source/, and /blame/ views.

As part of the implementation it adds a new cgit.js file
that is included in cgit page <head> along with a new
config "js" to specify its url, defaulting to "/cgit.js".

You can find the patches as a usable tree here

https://warmcat.com/git/cgit/log/

---

Andy Green (5):
      config: add js
      cgit.js: introduce
      ui-shared: introduce line range highlight javascript
      ui-tree: use the line range highlight script
      ui-blame: use the line range highlight script


 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.css     |    8 ++++++++
 cgit.h       |    1 +
 cgit.js      |   43 +++++++++++++++++++++++++++++++++++++++++++
 cgitrc.5.txt |    4 ++++
 ui-blame.c   |    1 +
 ui-shared.c  |   14 ++++++++++++++
 ui-shared.h  |    1 +
 ui-tree.c    |    2 ++
 10 files changed, 78 insertions(+)
 create mode 100644 cgit.js

--
Signature


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 1/5] config: add js
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
@ 2018-06-21  9:34   ` andy
  2018-06-23 10:20     ` john
  2018-06-21  9:34   ` [PATCH v2 2/5] cgit.js: introduce andy
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-21  9:34 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgitrc.5.txt |    4 ++++
 3 files changed, 8 insertions(+)

diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 99ea7a2..e5a703e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..a692aa5 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,10 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 2/5] cgit.js: introduce
  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-21  9:34   ` andy
  2018-06-23 10:18     ` john
  2018-06-21  9:34   ` [PATCH v2 3/5] ui-shared: introduce line range highlight javascript andy
                     ` (5 subsequent siblings)
  7 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-21  9:34 UTC (permalink / raw)


Similar to how cgit.css is handled, we will also provide and
reference a cgit.js for javascript from now on.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 Makefile |    1 +
 cgit.js  |    0 
 2 files changed, 1 insertion(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 229cd5d..083d1ce 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 3/5] ui-shared: introduce line range highlight javascript
  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-21  9:34   ` [PATCH v2 2/5] cgit.js: introduce andy
@ 2018-06-21  9:34   ` andy
  2018-06-23 10:17     ` john
  2018-06-21  9:35   ` [PATCH v2 4/5] ui-tree: use the line range highlight script andy
                     ` (4 subsequent siblings)
  7 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-21  9:34 UTC (permalink / raw)


This adds a small css class, a clientside js function in
cgit.js, and ajs inline script caller in ui-shared
functions to interpret the # part of the URL
on the client, and apply a highlight to filtered source.

Unlike blame highlight boxes which use generated divs, this
applied a computed absolute, transparent highlight over the
affected line(s) on the client side.

The # part of the URL is defined to not be passed to the server,
so the highlight can't be rendered on the server side.
However this has the advantage that the line range highlight
can operate on /blame/ urls trivially, since it doesn't
conflict with blame's generated div scheme.

pointer-events: none is used on the highlight overlay div to
allow the user to cut-and-paste in the highlit region and
click on links underneath normally.

The JS supports highlighting single lines as before like #n123
and also ranges of lines like #n123-135.

Because the browser can no longer automatically scroll to the
element in the second case, the JS also takes care of extracting
the range start element and scrolling to it dynamically.

Tested on Linux Firefox 60 + Linux Chrome 67

Examples:

https://warmcat.com/git/cgit/tree/ui-shared.c#n1146
https://warmcat.com/git/cgit/tree/ui-shared.c#n1146-1164
https://warmcat.com/git/cgit/blame/ui-shared.c#n1146-1164

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css    |    8 ++++++++
 cgit.js     |   43 +++++++++++++++++++++++++++++++++++++++++++
 ui-shared.c |   14 ++++++++++++++
 ui-shared.h |    1 +
 4 files changed, 66 insertions(+)

diff --git a/cgit.css b/cgit.css
index 61bbd49..7cb0fd6 100644
--- a/cgit.css
+++ b/cgit.css
@@ -368,6 +368,14 @@ div#cgit table.blame td.lines > div > pre {
 	top: 0;
 }
 
+div#cgit div.line_range {
+	position: absolute;
+	pointer-events: none;
+	left: 0px;
+	z-index: 1;
+	background-color: rgba(255, 255, 0, 0.2);
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index e69de29..6e3473c 100644
--- a/cgit.js
+++ b/cgit.js
@@ -0,0 +1,43 @@
+function cgit_line_range_highlight(depth)
+{
+	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
+
+	l1 = parseInt(h.substring(2));
+	t = h.indexOf("-");
+	if (t >= 1)
+		l2 = parseInt(h.substring(t + 1));
+	else
+		l2 = l1;
+
+	if (l1) {
+		var lh, t = 0, e1, de;
+
+		e1 = e = document.getElementById('n' + l1);
+		if (e) {
+
+			while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
+
+			de = document.createElement("DIV");
+
+			de.className = "line_range";
+			de.style.bottom = e.style.bottom;
+			de.style.top = t + 'px';
+			if (!depth)
+				de.style.width = e.parentElement.parentElement.parentNode.offsetWidth + 'px';
+			else
+				de.style.width = e.parentElement.parentElement.parentNode.parentNode.offsetWidth + 'px';
+			de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
+
+			e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
+
+			while (l1 <= l2) {
+				var e1;
+				e1 = document.getElementById('n' + l1++);
+				e1.style.backgroundColor = "yellow";
+			}
+
+			e.scrollIntoView(true);
+		}
+	}
+}
+
diff --git a/ui-shared.c b/ui-shared.c
index 082a6f1..7fd6bad 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -787,6 +787,10 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	html("<script type='text/javascript' src='");
+	html_attr(ctx.cfg.js);
+	html("'/></script>\n");
+
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);
@@ -1237,3 +1241,13 @@ void cgit_set_title_from_path(const char *path)
 	strcat(new_title, ctx.page.title);
 	ctx.page.title = new_title;
 }
+
+void cgit_emit_line_range_highlight_script(int parent_level)
+{
+	htmlf("<script>\n"
+	      "document.addEventListener(\"DOMContentLoaded\", function() {"
+	      "	cgit_line_range_highlight(%d);\n"
+	      "}, false);\n"
+	      "</script>\n", parent_level);
+}
+
diff --git a/ui-shared.h b/ui-shared.h
index e78b5fd..8b0cddc 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -89,4 +89,5 @@ extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
 				       const char *page);
 
 extern void cgit_set_title_from_path(const char *path);
+extern void cgit_emit_line_range_highlight_script(int parent_level);
 #endif /* UI_SHARED_H */



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 4/5] ui-tree: use the line range highlight script
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
                     ` (2 preceding siblings ...)
  2018-06-21  9:34   ` [PATCH v2 3/5] ui-shared: introduce line range highlight javascript andy
@ 2018-06-21  9:35   ` andy
  2018-06-21  9:35   ` [PATCH v2 5/5] ui-blame: " andy
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-21  9:35 UTC (permalink / raw)


This allows it to work in /tree/ and /source/ URLs.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 ui-tree.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..b66cabe 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -226,6 +226,8 @@ static void print_object(const struct object_id *oid, char *path, const char *ba
 	} else
 		print_buffer(basename, buf, size);
  
+	cgit_emit_line_range_highlight_script(0);
+
 	free(buf);
 	free(mimetype);
 }



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 5/5] ui-blame: use the line range highlight script
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
                     ` (3 preceding siblings ...)
  2018-06-21  9:35   ` [PATCH v2 4/5] ui-tree: use the line range highlight script andy
@ 2018-06-21  9:35   ` andy
  2018-06-22 23:01   ` [PATCH v2 1/2] cgit.js: make line range highlight responsive to url changes andy
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-21  9:35 UTC (permalink / raw)


This allows it to work on /blame/ URLs.

Blame has an extra layer of parent elements which
has to be taken into account by the JS when finding
the highlight width, hence the 1 arg to the emit
function.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 ui-blame.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..ef8a41b 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -212,6 +212,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	html("</tr>\n</table>\n");
 
+	cgit_emit_line_range_highlight_script(1);
 	cgit_print_layout_end();
 
 cleanup:



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 1/2] cgit.js: make line range highlight responsive to url changes
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
                     ` (4 preceding siblings ...)
  2018-06-21  9:35   ` [PATCH v2 5/5] ui-blame: " andy
@ 2018-06-22 23:01   ` 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
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-22 23:01 UTC (permalink / raw)


Default interpretation of the # part of the URL is to try to
match it to an element ID one time at page load.

Subsequent modifications in the URL bar are ignored, even if
you hit enter there.

This patch makes the line range highlight able to clean up
after itself and be reapplied, and has the highlight function
listen for hashchange events.

Now if you edit the URL and press enter, any existing highlight
is removed, the # part reinterpreted and the new highlight
applied automatically.

This is particularly useful if you edit the # link with the
intention to show a range before copying the URL.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   78 ++++++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/cgit.js b/cgit.js
index 6e3473c..5430c43 100644
--- a/cgit.js
+++ b/cgit.js
@@ -1,43 +1,69 @@
+var cgit_line_range_depth;
+
 function cgit_line_range_highlight(depth)
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	cgit_line_range_depth = depth;
+
+	e = document.getElementById("cgit_line_range");
+	if (e) {
+		l1 = e.l1;
+		while (l1 <= e.l2) {
+			var e1;
+			e1 = document.getElementById('n' + l1++);
+			e1.style.backgroundColor = null;
+		}
+
+		e.remove();
+	}
+
 	l1 = parseInt(h.substring(2));
+	if (!l1)
+		return;
+
 	t = h.indexOf("-");
-	if (t >= 1)
+	l2 = l1;
+	if (t >= 1) {
 		l2 = parseInt(h.substring(t + 1));
-	else
-		l2 = l1;
-
-	if (l1) {
-		var lh, t = 0, e1, de;
+		if (l2 < l1)
+			l2 = l1;
+	}
 
-		e1 = e = document.getElementById('n' + l1);
-		if (e) {
+	var lh, t = 0, e1, de;
 
-			while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
+	e1 = e = document.getElementById('n' + l1);
+	if (!e)
+		return;
 
-			de = document.createElement("DIV");
+	while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
 
-			de.className = "line_range";
-			de.style.bottom = e.style.bottom;
-			de.style.top = t + 'px';
-			if (!depth)
-				de.style.width = e.parentElement.parentElement.parentNode.offsetWidth + 'px';
-			else
-				de.style.width = e.parentElement.parentElement.parentNode.parentNode.offsetWidth + 'px';
-			de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
+	de = document.createElement("DIV");
 
-			e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
+	de.className = "line_range";
+	de.style.bottom = e.style.bottom;
+	de.style.top = t + 'px';
+	de.id = "cgit_line_range";
+	de.l1 = l1;
+	de.l2 = l2;
+	if (!depth)
+		de.style.width = e.parentElement.parentElement.parentNode.offsetWidth + 'px';
+	else
+		de.style.width = e.parentElement.parentElement.parentNode.parentNode.offsetWidth + 'px';
+	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
 
-			while (l1 <= l2) {
-				var e1;
-				e1 = document.getElementById('n' + l1++);
-				e1.style.backgroundColor = "yellow";
-			}
+	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
 
-			e.scrollIntoView(true);
-		}
+	while (l1 <= l2) {
+		var e1;
+		e1 = document.getElementById('n' + l1++);
+		e1.style.backgroundColor = "yellow";
 	}
+
+	window.addEventListener("hashchange", function() {
+		cgit_line_range_highlight(cgit_line_range_depth);
+	}, false);
+
+	e.scrollIntoView(true);
 }
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 2/2] cgit.js: line range highlight: improve vertical scroll logic
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
                     ` (5 preceding siblings ...)
  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   ` andy
  2018-06-23  7:45   ` [PATCH v2] cgit.js: line range highlight: always hook hashchange in case hash added andy
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-22 23:02 UTC (permalink / raw)


Instead of following the browser heuristic to put any matching
id element to the top left of the browser window, compute the
number of visible lines vertically in the window, and the
middle of the highlit range, and try to centre the middle of
the highlit range in the window.

If the top of the range is no longer visible due to a range
consisting of more lines than the window can show, fall back to
placing the top of the range at the top of the window.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/cgit.js b/cgit.js
index 5430c43..d905634 100644
--- a/cgit.js
+++ b/cgit.js
@@ -30,7 +30,7 @@ function cgit_line_range_highlight(depth)
 			l2 = l1;
 	}
 
-	var lh, t = 0, e1, de;
+	var lh, t = 0, e1, de, hl, v, n;
 
 	e1 = e = document.getElementById('n' + l1);
 	if (!e)
@@ -54,9 +54,10 @@ function cgit_line_range_highlight(depth)
 
 	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
 
-	while (l1 <= l2) {
+	n = l1;
+	while (n <= l2) {
 		var e1;
-		e1 = document.getElementById('n' + l1++);
+		e1 = document.getElementById('n' + n++);
 		e1.style.backgroundColor = "yellow";
 	}
 
@@ -64,6 +65,17 @@ function cgit_line_range_highlight(depth)
 		cgit_line_range_highlight(cgit_line_range_depth);
 	}, false);
 
-	e.scrollIntoView(true);
+	hl = (window.innerHeight / (e.offsetHeight + 1));
+	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
+	if (v > l1)
+		v = l1;
+	if (v < 0)
+		v = 1;
+		
+	e1 = document.getElementById('n' + Math.round(v));
+	if (e1)
+		e1.scrollIntoView(true);
+	else
+		e.scrollIntoView(true);
 }
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2] cgit.js: line range highlight: always hook hashchange in case hash added
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
                     ` (6 preceding siblings ...)
  2018-06-22 23:02   ` [PATCH v2 2/2] cgit.js: line range highlight: improve vertical scroll logic andy
@ 2018-06-23  7:45   ` andy
  7 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-23  7:45 UTC (permalink / raw)


Since the user might add a # to a URL that didn't initially have one,
eg by clicking the line numbers, always register the hashchange listener.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js     |    4 ----
 ui-shared.c |    5 ++++-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/cgit.js b/cgit.js
index d905634..a3ddf81 100644
--- a/cgit.js
+++ b/cgit.js
@@ -61,10 +61,6 @@ function cgit_line_range_highlight(depth)
 		e1.style.backgroundColor = "yellow";
 	}
 
-	window.addEventListener("hashchange", function() {
-		cgit_line_range_highlight(cgit_line_range_depth);
-	}, false);
-
 	hl = (window.innerHeight / (e.offsetHeight + 1));
 	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
 	if (v > l1)
diff --git a/ui-shared.c b/ui-shared.c
index 7fd6bad..3417919 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1245,9 +1245,12 @@ void cgit_set_title_from_path(const char *path)
 void cgit_emit_line_range_highlight_script(int parent_level)
 {
 	htmlf("<script>\n"
-	      "document.addEventListener(\"DOMContentLoaded\", function() {"
+	      "document.addEventListener(\"DOMContentLoaded\", function() {\n"
 	      "	cgit_line_range_highlight(%d);\n"
 	      "}, false);\n"
+	      "window.addEventListener(\"hashchange\", function() {\n"
+	      " cgit_line_range_highlight(%d);\n"
+	      "}, false);\n"
 	      "</script>\n", parent_level);
 }
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 3/5] ui-shared: introduce line range highlight javascript
  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
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-23 10:17 UTC (permalink / raw)


On Thu, Jun 21, 2018 at 05:34:59PM +0800, Andy Green wrote:
> This adds a small css class, a clientside js function in
> cgit.js, and ajs inline script caller in ui-shared
> functions to interpret the # part of the URL
> on the client, and apply a highlight to filtered source.
> 
> Unlike blame highlight boxes which use generated divs, this
> applied a computed absolute, transparent highlight over the
> affected line(s) on the client side.
> 
> The # part of the URL is defined to not be passed to the server,
> so the highlight can't be rendered on the server side.
> However this has the advantage that the line range highlight
> can operate on /blame/ urls trivially, since it doesn't
> conflict with blame's generated div scheme.
> 
> pointer-events: none is used on the highlight overlay div to
> allow the user to cut-and-paste in the highlit region and
> click on links underneath normally.
> 
> The JS supports highlighting single lines as before like #n123
> and also ranges of lines like #n123-135.
> 
> Because the browser can no longer automatically scroll to the
> element in the second case, the JS also takes care of extracting
> the range start element and scrolling to it dynamically.
> 
> Tested on Linux Firefox 60 + Linux Chrome 67
> 
> Examples:
> 
> https://warmcat.com/git/cgit/tree/ui-shared.c#n1146
> https://warmcat.com/git/cgit/tree/ui-shared.c#n1146-1164
> https://warmcat.com/git/cgit/blame/ui-shared.c#n1146-1164

These examples can move below the "---" since they don't need to be part
of the permanent Git history.

> Signed-off-by: Andy Green <andy at warmcat.com>
> ---
>  cgit.css    |    8 ++++++++
>  cgit.js     |   43 +++++++++++++++++++++++++++++++++++++++++++
>  ui-shared.c |   14 ++++++++++++++
>  ui-shared.h |    1 +
>  4 files changed, 66 insertions(+)
> 
> diff --git a/cgit.css b/cgit.css
> index 61bbd49..7cb0fd6 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -368,6 +368,14 @@ div#cgit table.blame td.lines > div > pre {
>  	top: 0;
>  }
>  
> +div#cgit div.line_range {

It looks like the normal convention for CSS classes is kebab-case rather
than snake_case.

I also wonder if the name should be "highlighed-lines" or
"selected-lines" or something like that to indicate that this isn't just
an arbitrary range of lines but ones that are being pulled out for some
reason.

> +	position: absolute;
> +	pointer-events: none;
> +	left: 0px;
> +	z-index: 1;
> +	background-color: rgba(255, 255, 0, 0.2);
> +}
> +
>  div#cgit table.bin-blob {
>  	margin-top: 0.5em;
>  	border: solid 1px black;
> diff --git a/cgit.js b/cgit.js
> index e69de29..6e3473c 100644
> --- a/cgit.js
> +++ b/cgit.js
> @@ -0,0 +1,43 @@
> +function cgit_line_range_highlight(depth)
> +{
> +	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
> +
> +	l1 = parseInt(h.substring(2));
> +	t = h.indexOf("-");
> +	if (t >= 1)
> +		l2 = parseInt(h.substring(t + 1));
> +	else
> +		l2 = l1;
> +
> +	if (l1) {
> +		var lh, t = 0, e1, de;
> +
> +		e1 = e = document.getElementById('n' + l1);
> +		if (e) {
> +
> +			while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
> +
> +			de = document.createElement("DIV");
> +
> +			de.className = "line_range";
> +			de.style.bottom = e.style.bottom;
> +			de.style.top = t + 'px';
> +			if (!depth)
> +				de.style.width = e.parentElement.parentElement.parentNode.offsetWidth + 'px';
> +			else
> +				de.style.width = e.parentElement.parentElement.parentNode.parentNode.offsetWidth + 'px';
> +			de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
> +
> +			e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
> +
> +			while (l1 <= l2) {
> +				var e1;
> +				e1 = document.getElementById('n' + l1++);
> +				e1.style.backgroundColor = "yellow";
> +			}
> +
> +			e.scrollIntoView(true);
> +		}
> +	}
> +}
> +
> diff --git a/ui-shared.c b/ui-shared.c
> index 082a6f1..7fd6bad 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -787,6 +787,10 @@ void cgit_print_docstart(void)
>  	html("<link rel='stylesheet' type='text/css' href='");
>  	html_attr(ctx.cfg.css);
>  	html("'/>\n");
> +	html("<script type='text/javascript' src='");
> +	html_attr(ctx.cfg.js);
> +	html("'/></script>\n");

This belongs in the previous patch which adds the .js file support.

>  	if (ctx.cfg.favicon) {
>  		html("<link rel='shortcut icon' href='");
>  		html_attr(ctx.cfg.favicon);
> @@ -1237,3 +1241,13 @@ void cgit_set_title_from_path(const char *path)
>  	strcat(new_title, ctx.page.title);
>  	ctx.page.title = new_title;
>  }
> +
> +void cgit_emit_line_range_highlight_script(int parent_level)
> +{
> +	htmlf("<script>\n"
> +	      "document.addEventListener(\"DOMContentLoaded\", function() {"
> +	      "	cgit_line_range_highlight(%d);\n"
> +	      "}, false);\n"
> +	      "</script>\n", parent_level);

Can we do this unconditionally in the .js file?

Since it handles the case where the target line element isn't found, I
don't think it will cause problems to run this on every page.

> +}
> +
> diff --git a/ui-shared.h b/ui-shared.h
> index e78b5fd..8b0cddc 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -89,4 +89,5 @@ extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
>  				       const char *page);
>  
>  extern void cgit_set_title_from_path(const char *path);
> +extern void cgit_emit_line_range_highlight_script(int parent_level);
>  #endif /* UI_SHARED_H */


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 2/5] cgit.js: introduce
  2018-06-21  9:34   ` [PATCH v2 2/5] cgit.js: introduce andy
@ 2018-06-23 10:18     ` john
  0 siblings, 0 replies; 73+ messages in thread
From: john @ 2018-06-23 10:18 UTC (permalink / raw)


On Thu, Jun 21, 2018 at 05:34:54PM +0800, Andy Green wrote:
> Similar to how cgit.css is handled, we will also provide and
> reference a cgit.js for javascript from now on.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> ---

I think we can merge this and the previous patch, along with the change
to output a <script> element in the document header.

>  Makefile |    1 +
>  cgit.js  |    0 
>  2 files changed, 1 insertion(+)
>  create mode 100644 cgit.js
> 
> diff --git a/Makefile b/Makefile
> index 229cd5d..083d1ce 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -87,6 +87,7 @@ install: all
>  	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
>  	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
>  	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
> +	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
>  	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
>  	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
>  	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
> diff --git a/cgit.js b/cgit.js
> new file mode 100644
> index 0000000..e69de29


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 1/5] config: add js
  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
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-23 10:20 UTC (permalink / raw)


On Thu, Jun 21, 2018 at 05:34:49PM +0800, Andy Green wrote:
> Just like the config allows setting css URL path,
> add a config for setting the js URL path
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> ---
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 99fc799..a692aa5 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -248,6 +248,10 @@ inline-readme::
>  	individually also choose to ignore this global list, and create a
>  	repo-specific list by using 'repo.inline-readme'.
>  
> +js::
> +	Url which specifies the javascript script document to include in all cgit
> +	pages.  Default value: "/cgit.js".

Should we allow the empty string to disable this option?

>  local-time::
>  	Flag which, if set to "1", makes cgit print commit and tag times in the
>  	servers timezone. Default value: "0".
> 


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 1/5] config: add js
  2018-06-23 10:20     ` john
@ 2018-06-23 10:34       ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-23 10:34 UTC (permalink / raw)




On 06/23/2018 06:20 PM, John Keeping wrote:
> On Thu, Jun 21, 2018 at 05:34:49PM +0800, Andy Green wrote:
>> Just like the config allows setting css URL path,
>> add a config for setting the js URL path
>>
>> Signed-off-by: Andy Green <andy at warmcat.com>
>> ---
>> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
>> index 99fc799..a692aa5 100644
>> --- a/cgitrc.5.txt
>> +++ b/cgitrc.5.txt
>> @@ -248,6 +248,10 @@ inline-readme::
>>   	individually also choose to ignore this global list, and create a
>>   	repo-specific list by using 'repo.inline-readme'.
>>   
>> +js::
>> +	Url which specifies the javascript script document to include in all cgit
>> +	pages.  Default value: "/cgit.js".
> 
> Should we allow the empty string to disable this option?

OK.  It will additionally disable any feature then that wants to use 
functions from cgit.js.

-Andy

>>   local-time::
>>   	Flag which, if set to "1", makes cgit print commit and tag times in the
>>   	servers timezone. Default value: "0".
>>


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v2 3/5] ui-shared: introduce line range highlight javascript
  2018-06-23 10:17     ` john
@ 2018-06-24  2:37       ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-24  2:37 UTC (permalink / raw)




On 06/23/2018 06:17 PM, John Keeping wrote:
> On Thu, Jun 21, 2018 at 05:34:59PM +0800, Andy Green wrote:
>> This adds a small css class, a clientside js function in
>> cgit.js, and ajs inline script caller in ui-shared
>> functions to interpret the # part of the URL
>> on the client, and apply a highlight to filtered source.
>>
>> Unlike blame highlight boxes which use generated divs, this
>> applied a computed absolute, transparent highlight over the
>> affected line(s) on the client side.
>>
>> The # part of the URL is defined to not be passed to the server,
>> so the highlight can't be rendered on the server side.
>> However this has the advantage that the line range highlight
>> can operate on /blame/ urls trivially, since it doesn't
>> conflict with blame's generated div scheme.
>>
>> pointer-events: none is used on the highlight overlay div to
>> allow the user to cut-and-paste in the highlit region and
>> click on links underneath normally.
>>
>> The JS supports highlighting single lines as before like #n123
>> and also ranges of lines like #n123-135.
>>
>> Because the browser can no longer automatically scroll to the
>> element in the second case, the JS also takes care of extracting
>> the range start element and scrolling to it dynamically.
>>
>> Tested on Linux Firefox 60 + Linux Chrome 67
>>
>> Examples:
>>
>> https://warmcat.com/git/cgit/tree/ui-shared.c#n1146
>> https://warmcat.com/git/cgit/tree/ui-shared.c#n1146-1164
>> https://warmcat.com/git/cgit/blame/ui-shared.c#n1146-1164

They better belong in a series cover letter.

> These examples can move below the "---" since they don't need to be part
> of the permanent Git history.
> 
>> Signed-off-by: Andy Green <andy at warmcat.com>
>> ---
>>   cgit.css    |    8 ++++++++
>>   cgit.js     |   43 +++++++++++++++++++++++++++++++++++++++++++
>>   ui-shared.c |   14 ++++++++++++++
>>   ui-shared.h |    1 +
>>   4 files changed, 66 insertions(+)
>>
>> diff --git a/cgit.css b/cgit.css
>> index 61bbd49..7cb0fd6 100644
>> --- a/cgit.css
>> +++ b/cgit.css
>> @@ -368,6 +368,14 @@ div#cgit table.blame td.lines > div > pre {
>>   	top: 0;
>>   }
>>   
>> +div#cgit div.line_range {
> 
> It looks like the normal convention for CSS classes is kebab-case rather
> than snake_case.
> 
> I also wonder if the name should be "highlighed-lines" or
> "selected-lines" or something like that to indicate that this isn't just
> an arbitrary range of lines but ones that are being pulled out for some
> reason.

OK, "selected-lines" then.

>> diff --git a/ui-shared.c b/ui-shared.c
>> index 082a6f1..7fd6bad 100644
>> --- a/ui-shared.c
>> +++ b/ui-shared.c
>> @@ -787,6 +787,10 @@ void cgit_print_docstart(void)
>>   	html("<link rel='stylesheet' type='text/css' href='");
>>   	html_attr(ctx.cfg.css);
>>   	html("'/>\n");
>> +	html("<script type='text/javascript' src='");
>> +	html_attr(ctx.cfg.js);
>> +	html("'/></script>\n");
> 
> This belongs in the previous patch which adds the .js file support.

OK, they got squashed as requested as well.

>>   	if (ctx.cfg.favicon) {
>>   		html("<link rel='shortcut icon' href='");
>>   		html_attr(ctx.cfg.favicon);
>> @@ -1237,3 +1241,13 @@ void cgit_set_title_from_path(const char *path)
>>   	strcat(new_title, ctx.page.title);
>>   	ctx.page.title = new_title;
>>   }
>> +
>> +void cgit_emit_line_range_highlight_script(int parent_level)
>> +{
>> +	htmlf("<script>\n"
>> +	      "document.addEventListener(\"DOMContentLoaded\", function() {"
>> +	      "	cgit_line_range_highlight(%d);\n"
>> +	      "}, false);\n"
>> +	      "</script>\n", parent_level);
> 
> Can we do this unconditionally in the .js file?

Yes... but it's complicated by having to rediscover if we are in blame 
or tree only by looking at the DOM (since we don't know our server 
virtual path in the client, that's not simple to reliably extract from 
the URL), rather than in the C having complete and reliable knowledge of 
if we are generating blame or tree.  But a simple heuristic works for 
that in the JS by looking at the DOM atm.

> Since it handles the case where the target line element isn't found, I
> don't think it will cause problems to run this on every page.

I changed it to do so.

-Andy


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 0/6] line range highlight
  2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
                   ` (3 preceding siblings ...)
  2018-06-21  9:34 ` [PATCH v2 0/5] line range highlight andy
@ 2018-06-24  2:44 ` andy
  2018-06-24  2:44   ` [PATCH v3 1/6] config: add js andy
                     ` (5 more replies)
  2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
                   ` (2 subsequent siblings)
  7 siblings, 6 replies; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


The following series adds the ability to direct the browser
to highlight specific lines and ranges of lines in
/tree/, /source/, and /blame/ views, using the existing
#URLs.

As part of the implementation it adds a new cgit.js file
that is included in cgit page <head> along with a new
config "js" to specify its url, defaulting to "/cgit.js".
It can be disabled by setting the "js" config to an empty
string.

Since v2 the footprint in the C has been reduced to a few
lines; the user is able to define ranges in the browser
by clicking on the line numbers alone; and clicking on the
line numbers auto-copies the line or range URL to the
clipboard.

You can find the patches as a usable tree here

https://warmcat.com/git/cgit/log/

Examples:

https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n152
https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n39-47

---

Andy Green (6):
      config: add js
      ui-shared: line range highlight: introduce javascript
      cgit.js: line range highlight: make responsive to url changes
      cgit.js: line range highlight: improve vertical scroll logic
      line-range-highlight: onclick handler and range selection
      line-range-highlight: copy URL to clipboard on click


 Makefile     |    1 
 cgit.c       |    3 +
 cgit.css     |    7 +++
 cgit.h       |    1 
 cgit.js      |  158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 cgitrc.5.txt |    6 ++
 ui-blame.c   |    2 -
 ui-shared.c  |    5 ++
 ui-tree.c    |    2 -
 9 files changed, 183 insertions(+), 2 deletions(-)
 create mode 100644 cgit.js

--
Signature


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 1/6] config: add js
  2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
@ 2018-06-24  2:44   ` andy
  2018-06-24 11:01     ` john
  2018-06-24  2:44   ` [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript andy
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Setting the js path to an empty string disables
emitting the reference to it in the head section.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgit.js      |    0 
 cgitrc.5.txt |    6 ++++++
 ui-shared.c  |    5 +++++
 6 files changed, 16 insertions(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 137150c..de7e13e 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 99ea7a2..e5a703e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..bdd799f 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,12 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".  Setting this to an empty string will
+	disable the link to this file in the head section and defeat generation
+	of any features needing functions from it.
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".
diff --git a/ui-shared.c b/ui-shared.c
index 082a6f1..e2f3d55 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -787,6 +787,11 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	if (*ctx.cfg.js) {
+		html("<script type='text/javascript' src='");
+		html_attr(ctx.cfg.js);
+		html("'/></script>\n");
+	}
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript
  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  2:44   ` andy
  2018-06-24 11:28     ` john
  2018-06-24  2:44   ` [PATCH v3 3/6] cgit.js: line range highlight: make responsive to url changes andy
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


This adds a small css class, and a clientside js function plus
event registrations in cgit.js, to interpret the # part of the
URL on the client, and apply a highlight to filtered source.

Unlike blame highlight boxes which use generated divs, this
applies a computed absolutely-positioned, transparent div highlight
over the affected line(s) on the client side.

The # part of the URL is defined to not be passed to the server,
so the highlight can't be rendered on the server side.
However this has the advantage that the line range highlight
can operate on /blame/ urls trivially, since it doesn't
conflict with blame's generated div scheme.

pointer-events: none is used on the highlight overlay div to
allow the user to cut-and-paste in the highlit region and
click on links underneath normally.

The JS supports highlighting single lines as before like #n123
and also ranges of lines like #n123-135.

Because the browser can no longer automatically scroll to the
element in the second case, the JS also takes care of extracting
the range start element and scrolling to it dynamically.

Tested on Linux Firefox 60 + Linux Chrome 67

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |    7 +++++++
 cgit.js  |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/cgit.css b/cgit.css
index 61bbd49..443e04c 100644
--- a/cgit.css
+++ b/cgit.css
@@ -368,6 +368,13 @@ div#cgit table.blame td.lines > div > pre {
 	top: 0;
 }
 
+div#cgit div.selected-lines {
+	position: absolute;
+	pointer-events: none;
+	z-index: 1;
+	background-color: rgba(255, 255, 0, 0.2);
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index e69de29..501c98f 100644
--- a/cgit.js
+++ b/cgit.js
@@ -0,0 +1,53 @@
+function cgit_line_range_highlight()
+{
+	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
+
+	l1 = parseInt(h.substring(2));
+	t = h.indexOf("-");
+	if (t >= 1)
+		l2 = parseInt(h.substring(t + 1));
+	else
+		l2 = l1;
+
+	if (!l1)
+		return;
+
+	var lh, t = 0, e1, e2, de;
+
+	e1 = e = document.getElementById('n' + l1);
+	if (!e)
+		return;
+
+	while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
+
+	de = document.createElement("DIV");
+
+	de.className = "selected-lines";
+	de.style.bottom = e.style.bottom;
+	de.style.top = t + 'px';
+
+	/* DOM structure is different by one level for /blame/ */
+	e1 = e.parentElement.parentElement.parentNode;
+	if (e1.offsetWidth < e1.parentNode.offsetWidth) /* blame */
+		e1 = e1.parentNode;
+
+	de.style.width = e1.offsetWidth + 'px';
+	de.style.left = e1.parentNode.parentNode.offsetLeft + 'px';
+
+	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
+
+	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
+
+	while (l1 <= l2) {
+		e1 = document.getElementById('n' + l1++);
+		e1.style.backgroundColor = "yellow";
+	}
+
+	e.scrollIntoView(true);
+}
+
+/* line range highlight */
+
+document.addEventListener("DOMContentLoaded", function() {
+	cgit_line_range_highlight();
+}, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 3/6] cgit.js: line range highlight: make responsive to url changes
  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  2:44   ` [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript andy
@ 2018-06-24  2:44   ` andy
  2018-06-24  2:44   ` [PATCH v3 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


Browser default interpretation of the # part of the URL is
to try to match it to an element ID one time at page load.

Subsequent modifications in the URL bar are ignored, even if
you hit enter there.

This patch makes the line range highlight able to clean up
after itself and be reapplied, and has the highlight function
listen for hashchange events and re-interpret the # part
when they occur.

Now if you edit the URL and press enter, any existing highlight
is removed, the # part reinterpreted and the new highlight
applied automatically.

This is particularly useful if you edit the # link with the
intention to show a range before copying the URL.

Clicking on the line number, which changes the URL bar to
have a corresponding #n part, also triggers hashchange and
makes the clicked line be highlit immediately.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/cgit.js b/cgit.js
index 501c98f..772843a 100644
--- a/cgit.js
+++ b/cgit.js
@@ -2,8 +2,24 @@ function cgit_line_range_highlight()
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	e = document.getElementById("cgit_line_range");
+	if (e) {
+		l1 = e.l1;
+		while (l1 <= e.l2) {
+			var e1;
+			e1 = document.getElementById('n' + l1++);
+			e1.style.backgroundColor = null;
+		}
+
+		e.remove();
+	}
+
 	l1 = parseInt(h.substring(2));
+	if (!l1)
+		return;
+
 	t = h.indexOf("-");
+	l2 = l1;
 	if (t >= 1)
 		l2 = parseInt(h.substring(t + 1));
 	else
@@ -12,6 +28,9 @@ function cgit_line_range_highlight()
 	if (!l1)
 		return;
 
+	if (l2 < l1)
+		l2 = l1;
+
 	var lh, t = 0, e1, e2, de;
 
 	e1 = e = document.getElementById('n' + l1);
@@ -25,6 +44,9 @@ function cgit_line_range_highlight()
 	de.className = "selected-lines";
 	de.style.bottom = e.style.bottom;
 	de.style.top = t + 'px';
+	de.id = "cgit_line_range";
+	de.l1 = l1;
+	de.l2 = l2;
 
 	/* DOM structure is different by one level for /blame/ */
 	e1 = e.parentElement.parentElement.parentNode;
@@ -39,6 +61,7 @@ function cgit_line_range_highlight()
 	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
 
 	while (l1 <= l2) {
+		var e1;
 		e1 = document.getElementById('n' + l1++);
 		e1.style.backgroundColor = "yellow";
 	}
@@ -51,3 +74,6 @@ function cgit_line_range_highlight()
 document.addEventListener("DOMContentLoaded", function() {
 	cgit_line_range_highlight();
 }, false);
+window.addEventListener("hashchange", function() {
+	cgit_line_range_highlight();
+}, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 4/6] cgit.js: line range highlight: improve vertical scroll logic
  2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
                     ` (2 preceding siblings ...)
  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   ` andy
  2018-06-24  2:44   ` [PATCH v3 5/6] line-range-highlight: onclick handler and range selection andy
  2018-06-24  2:44   ` [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click andy
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


Instead of following the browser heuristic to put any matching
id element to the top left of the browser window, compute the
number of visible lines vertically in the window, and the
middle of the highlit range, and try to centre the middle of
the highlit range in the window.

If the top of the range is no longer visible due to a range
consisting of more lines than the window can show, fall back to
placing the top of the range at the top of the window.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/cgit.js b/cgit.js
index 772843a..29b2c3d 100644
--- a/cgit.js
+++ b/cgit.js
@@ -31,7 +31,7 @@ function cgit_line_range_highlight()
 	if (l2 < l1)
 		l2 = l1;
 
-	var lh, t = 0, e1, e2, de;
+	var lh, t = 0, e1, e2, de, hl, v, n;
 
 	e1 = e = document.getElementById('n' + l1);
 	if (!e)
@@ -60,13 +60,25 @@ function cgit_line_range_highlight()
 
 	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
 
-	while (l1 <= l2) {
+	n = l1;
+	while (n <= l2) {
 		var e1;
-		e1 = document.getElementById('n' + l1++);
+		e1 = document.getElementById('n' + n++);
 		e1.style.backgroundColor = "yellow";
 	}
 
-	e.scrollIntoView(true);
+	hl = (window.innerHeight / (e.offsetHeight + 1));
+	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
+	if (v > l1)
+		v = l1;
+	if (v < 0)
+		v = 1;
+
+	e1 = document.getElementById('n' + Math.round(v));
+	if (e1)
+		e1.scrollIntoView(true);
+	else
+		e.scrollIntoView(true);
 }
 
 /* line range highlight */



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 5/6] line-range-highlight: onclick handler and range selection
  2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
                     ` (3 preceding siblings ...)
  2018-06-24  2:44   ` [PATCH v3 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
@ 2018-06-24  2:44   ` andy
  2018-06-24 11:35     ` john
  2018-06-24  2:44   ` [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click andy
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


This allows the user to select line ranges simply by clicking on the
line number links.

 - No selected highlit line, or a range already selected, causes the
click to highlight just the clicked line as usual.

 - Clicking on a second line number link when a single line was already
highlit creates a line range highlight, between the lowest and
highest line numbers of the already-selected and newly-selected
line number links.

The order of the clicks is unimportant, you can click the higher
line number link first and then the lower to define the range of
lines equally well.

The implementation is slightly complicated by our single parent
onclick handler not being able to interrupt the already ongoing
processing of the a href #n change from the link click itself.

Rather than bloat every linenumber link with its own onclick handler
defeating this default we simply do it with a single parent
onclick event and apply any computed range url in the existing
hashchange event handler.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js    |   39 +++++++++++++++++++++++++++++++++++++++
 ui-blame.c |    2 +-
 ui-tree.c  |    2 +-
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index 29b2c3d..2cfad29 100644
--- a/cgit.js
+++ b/cgit.js
@@ -1,7 +1,15 @@
+var cgit_line_range_override;
+
 function cgit_line_range_highlight()
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	if (cgit_line_range_override) {
+		window.location = cgit_line_range_override;
+		cgit_line_range_override = null;
+		return;
+	}
+
 	e = document.getElementById("cgit_line_range");
 	if (e) {
 		l1 = e.l1;
@@ -81,10 +89,41 @@ function cgit_line_range_highlight()
 		e.scrollIntoView(true);
 }
 
+function cgit_line_range_click(e) {
+	var t = e.target.id;
+
+	cgit_line_range_override = null;
+
+	/*
+	 * We are called while window location update from the
+	 * click on the #n link is underway.  So stash any
+	 * computed range #url for when the hashchange hander
+	 * is called, and override it there.
+	 */
+
+	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 =
+				window.location + '-' + t.substring(1);
+		else /* backwards */
+			cgit_line_range_override =
+				window.location.href.substring(0,
+					window.location.href.length -
+					window.location.hash.length) +
+				'#n' + t.substring(1) + '-' +
+					window.location.href.substring(
+						window.location.href.length -
+						window.location.hash.length + 2);
+}
+
 /* line range highlight */
 
 document.addEventListener("DOMContentLoaded", function() {
 	cgit_line_range_highlight();
+	var e = document.getElementById("linenumber_td");
+	if (e)
+		e.addEventListener("click", cgit_line_range_click, true);
 }, false);
 window.addEventListener("hashchange", function() {
 	cgit_line_range_highlight();
diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..0ba8a5a 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	/* Line numbers */
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<td class='linenumbers'>");
+		html("<td id=linenumber_td class='linenumbers'>");
 		for (ent = sb.ent; ent; ent = ent->next) {
 			html("<div class='alt'><pre>");
 			emit_blame_entry_linenumber(ent);
diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..6759b11 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
 	html("<table summary='blob content' class='blob'>\n");
 
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<tr><td class='linenumbers'><pre>");
+		html("<tr><td id=linenumber_td class='linenumbers'><pre>");
 		idx = 0;
 		lineno = 0;
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
                     ` (4 preceding siblings ...)
  2018-06-24  2:44   ` [PATCH v3 5/6] line-range-highlight: onclick handler and range selection andy
@ 2018-06-24  2:44   ` andy
  2018-06-24 11:42     ` john
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-24  2:44 UTC (permalink / raw)


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.

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 */



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 1/6] config: add js
  2018-06-24  2:44   ` [PATCH v3 1/6] config: add js andy
@ 2018-06-24 11:01     ` john
  0 siblings, 0 replies; 73+ messages in thread
From: john @ 2018-06-24 11:01 UTC (permalink / raw)


On Sun, Jun 24, 2018 at 10:44:29AM +0800, Andy Green wrote:
> Just like the config allows setting css URL path,
> add a config for setting the js URL path
> 
> Setting the js path to an empty string disables
> emitting the reference to it in the head section.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>

For the code changes,

Reviewed-by: John Keeping <john at keeping.me.uk>

One comment on the documentation below...

> ---
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 99fc799..bdd799f 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -248,6 +248,12 @@ inline-readme::
>  	individually also choose to ignore this global list, and create a
>  	repo-specific list by using 'repo.inline-readme'.
>  
> +js::
> +	Url which specifies the javascript script document to include in all cgit
> +	pages.  Default value: "/cgit.js".  Setting this to an empty string will
> +	disable the link to this file in the head section and defeat generation
> +	of any features needing functions from it.

In this version of the series, I don't think we generate anythink that
calls into JS, so can this last clause be deleted?

>  local-time::
>  	Flag which, if set to "1", makes cgit print commit and tag times in the
>  	servers timezone. Default value: "0".


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript
  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
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-24 11:28 UTC (permalink / raw)


The subject is "ui-shared: ..." but should be "cgit.js" now I think.

On Sun, Jun 24, 2018 at 10:44:34AM +0800, Andy Green wrote:
> diff --git a/cgit.js b/cgit.js
> index e69de29..501c98f 100644
> --- a/cgit.js
> +++ b/cgit.js
> @@ -0,0 +1,53 @@
> +function cgit_line_range_highlight()
> +{
> +	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
> +
> +	l1 = parseInt(h.substring(2));
> +	t = h.indexOf("-");
> +	if (t >= 1)
> +		l2 = parseInt(h.substring(t + 1));
> +	else
> +		l2 = l1;
> +
> +	if (!l1)
> +		return;
> +
> +	var lh, t = 0, e1, e2, de;
> +
> +	e1 = e = document.getElementById('n' + l1);
> +	if (!e)
> +		return;
> +
> +	while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
> +
> +	de = document.createElement("DIV");
> +
> +	de.className = "selected-lines";
> +	de.style.bottom = e.style.bottom;
> +	de.style.top = t + 'px';
> +
> +	/* DOM structure is different by one level for /blame/ */
> +	e1 = e.parentElement.parentElement.parentNode;
> +	if (e1.offsetWidth < e1.parentNode.offsetWidth) /* blame */
> +		e1 = e1.parentNode;

I guess this is trying to find the <tr> element, so why not look for
that directly?

	while (e1.tagName.toUpperCase() != 'TR')
		e1 = e1.parentNode;

> +	de.style.width = e1.offsetWidth + 'px';
> +	de.style.left = e1.parentNode.parentNode.offsetLeft + 'px';

Again, if we're looking for the <table> here, can we make this more
robust by looking for it explicitly?

> +	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
> +
> +	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);

If I'm following correctly, e.parentElement.parentElement is either <td>
in tree view or <div> in blame view.  Is that expected?

My thinking is that it is correct, but what we're looking for is the
parent of the <pre> element containing the <a> with the line number ID.
Some comments would really help make sense of these chains of parent
references!

> +	while (l1 <= l2) {
> +		e1 = document.getElementById('n' + l1++);
> +		e1.style.backgroundColor = "yellow";
> +	}
> +
> +	e.scrollIntoView(true);
> +}
> +
> +/* line range highlight */
> +
> +document.addEventListener("DOMContentLoaded", function() {
> +	cgit_line_range_highlight();
> +}, false);


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 5/6] line-range-highlight: onclick handler and range selection
  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
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-24 11:35 UTC (permalink / raw)


On Sun, Jun 24, 2018 at 10:44:49AM +0800, Andy Green wrote:
> This allows the user to select line ranges simply by clicking on the
> line number links.
> 
>  - No selected highlit line, or a range already selected, causes the
> click to highlight just the clicked line as usual.
> 
>  - Clicking on a second line number link when a single line was already
> highlit creates a line range highlight, between the lowest and
> highest line numbers of the already-selected and newly-selected
> line number links.
> 
> The order of the clicks is unimportant, you can click the higher
> line number link first and then the lower to define the range of
> lines equally well.
> 
> The implementation is slightly complicated by our single parent
> onclick handler not being able to interrupt the already ongoing
> processing of the a href #n change from the link click itself.
> 
> Rather than bloat every linenumber link with its own onclick handler
> defeating this default we simply do it with a single parent
> onclick event and apply any computed range url in the existing
> hashchange event handler.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> ---
>  cgit.js    |   39 +++++++++++++++++++++++++++++++++++++++
>  ui-blame.c |    2 +-
>  ui-tree.c  |    2 +-
>  3 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/cgit.js b/cgit.js
> index 29b2c3d..2cfad29 100644
> --- a/cgit.js
> +++ b/cgit.js
> @@ -1,7 +1,15 @@
> +var cgit_line_range_override;
> +
>  function cgit_line_range_highlight()
>  {
>  	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
>  
> +	if (cgit_line_range_override) {
> +		window.location = cgit_line_range_override;
> +		cgit_line_range_override = null;
> +		return;
> +	}
> +
>  	e = document.getElementById("cgit_line_range");
>  	if (e) {
>  		l1 = e.l1;
> @@ -81,10 +89,41 @@ function cgit_line_range_highlight()
>  		e.scrollIntoView(true);
>  }
>  
> +function cgit_line_range_click(e) {
> +	var t = e.target.id;
> +
> +	cgit_line_range_override = null;
> +
> +	/*
> +	 * We are called while window location update from the
> +	 * click on the #n link is underway.  So stash any
> +	 * computed range #url for when the hashchange hander
> +	 * is called, and override it there.
> +	 */
> +
> +	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 =
> +				window.location + '-' + t.substring(1);
> +		else /* backwards */
> +			cgit_line_range_override =
> +				window.location.href.substring(0,
> +					window.location.href.length -
> +					window.location.hash.length) +
> +				'#n' + t.substring(1) + '-' +
> +					window.location.href.substring(
> +						window.location.href.length -
> +						window.location.hash.length + 2);
> +}
> +
>  /* line range highlight */
>  
>  document.addEventListener("DOMContentLoaded", function() {
>  	cgit_line_range_highlight();
> +	var e = document.getElementById("linenumber_td");
> +	if (e)
> +		e.addEventListener("click", cgit_line_range_click, true);
>  }, false);
>  window.addEventListener("hashchange", function() {
>  	cgit_line_range_highlight();
> diff --git a/ui-blame.c b/ui-blame.c
> index 50d0580..0ba8a5a 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
>  
>  	/* Line numbers */
>  	if (ctx.cfg.enable_tree_linenumbers) {
> -		html("<td class='linenumbers'>");
> +		html("<td id=linenumber_td class='linenumbers'>");

We normally quote attribute values in all cases (like for "class" here),
and I think we choose kebab-case over snake_case for HTML identifiers,
by the same principle that we do so for CSS classes.

But can we just call this "linenumbers"?  I don't think we need the
element type in the identifier.

>  		for (ent = sb.ent; ent; ent = ent->next) {
>  			html("<div class='alt'><pre>");
>  			emit_blame_entry_linenumber(ent);
> diff --git a/ui-tree.c b/ui-tree.c
> index e4cb558..6759b11 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
>  	html("<table summary='blob content' class='blob'>\n");
>  
>  	if (ctx.cfg.enable_tree_linenumbers) {
> -		html("<tr><td class='linenumbers'><pre>");
> +		html("<tr><td id=linenumber_td class='linenumbers'><pre>");
>  		idx = 0;
>  		lineno = 0;
>  


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  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
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-24 11:42 UTC (permalink / raw)


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.

> 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


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  2018-06-24 11:42     ` john
@ 2018-06-24 12:00       ` andy
  2018-06-24 13:39         ` john
  0 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-24 12:00 UTC (permalink / raw)




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?

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?

-Andy

>> 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


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  2018-06-24 12:00       ` andy
@ 2018-06-24 13:39         ` john
  2018-06-24 15:06           ` andy
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-24 13:39 UTC (permalink / raw)


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


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  2018-06-24 13:39         ` john
@ 2018-06-24 15:06           ` andy
  2018-06-24 16:03             ` john
  0 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-24 15:06 UTC (permalink / raw)




On June 24, 2018 9:39:35 PM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
>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.

Well, ok, so he can click on it to show his buddy.  He gets the URL in his clipboard.  But his buddy is there he doesn't need it.

>> 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

The browser has no idea about that.  Cgit isn't an X11 beast, it feeds crossplatform browsers.

If the browser copies anything to the clipboard by js api, it's always the ^C ^V one since the js stuff only has the concept of that one.  So in the case the user wants this behaviour, the surprising thing would be on some platforms it went in some other "clipboard" (this is not possible in js afaik).  The user would be nonplussed if it could and then he was sat in front of a windows browser.  So I think the observations about that lead nowhere.

>hyperlink fails the principle of least surprise, and I think that

Well, I used it and I liked it.  It felt pretty natural.  But an extra click won't kill me.

>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.

On github it pops up a sticky popup with an ellipsis because you "clicked a hyperlink" in the same conditions.  (Clicking that brings a js menu with a copy entry).  Users can handle variation, at least if it's indicating to them what's happening and why, and is useful.  At least it doesn't seem to have held github back...

The link we want to copy is not the ...#nXXX link in the a href when there is a range up.  I never tried to intercept "Copy Link", maybe it could be done.  But at least the default menu won't do as it is in this case.

I think no point for the two-step menu like github since the only option we plan to show atm is Copy (the other useful option is switch to blame at that line).  How about a popup that appears on the left for a few secs with a copy type icon, or "Copy", in it, that fades away if not used?  It's one extra click but still one less than github.

-Andy

>[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


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  2018-06-24 15:06           ` andy
@ 2018-06-24 16:03             ` john
  2018-06-25  0:46               ` andy
  0 siblings, 1 reply; 73+ messages in thread
From: john @ 2018-06-24 16:03 UTC (permalink / raw)


On Sun, Jun 24, 2018 at 11:06:45PM +0800, Andy Green wrote:
> On June 24, 2018 9:39:35 PM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
> >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.
> 
> Well, ok, so he can click on it to show his buddy.  He gets the URL in
> his clipboard.  But his buddy is there he doesn't need it.

And what about the piece of code that was cut into the clipboard just
before the interruption?

> >> 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
> 
> The browser has no idea about that.  Cgit isn't an X11 beast, it feeds
> crossplatform browsers.
> 
> If the browser copies anything to the clipboard by js api, it's always
> the ^C ^V one since the js stuff only has the concept of that one.  So
> in the case the user wants this behaviour, the surprising thing would
> be on some platforms it went in some other "clipboard" (this is not
> possible in js afaik).  The user would be nonplussed if it could and
> then he was sat in front of a windows browser.  So I think the
> observations about that lead nowhere.
> 
> >hyperlink fails the principle of least surprise, and I think that
> 
> Well, I used it and I liked it.  It felt pretty natural.  But an extra
> click won't kill me.
> 
> >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.
> 
> On github it pops up a sticky popup with an ellipsis because you
> "clicked a hyperlink" in the same conditions.  (Clicking that brings a
> js menu with a copy entry).  Users can handle variation, at least if
> it's indicating to them what's happening and why, and is useful.  At
> least it doesn't seem to have held github back...

That's fine, it doesn't overwrite the clipboard with no notice (and
right-click still works).  My point isn't "we can't provide a new way to
copy to the clipboard", it's "we can't overwrite the clipboard behind
the user's back".

> The link we want to copy is not the ...#nXXX link in the a href when
> there is a range up.  I never tried to intercept "Copy Link", maybe it
> could be done.  But at least the default menu won't do as it is in
> this case.
> 
> I think no point for the two-step menu like github since the only
> option we plan to show atm is Copy (the other useful option is switch
> to blame at that line).  How about a popup that appears on the left
> for a few secs with a copy type icon, or "Copy", in it, that fades
> away if not used?  It's one extra click but still one less than
> github.

This sounds good.  Copying the line range link is clearly useful and
this way the user knows that it's an option and controls when it
happens.

> >[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


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
  2018-06-24 16:03             ` john
@ 2018-06-25  0:46               ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  0:46 UTC (permalink / raw)




On June 25, 2018 12:03:56 AM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
>On Sun, Jun 24, 2018 at 11:06:45PM +0800, Andy Green wrote:
>> On June 24, 2018 9:39:35 PM GMT+08:00, John Keeping
><john at keeping.me.uk> wrote:
>> >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.
>> 
>> Well, ok, so he can click on it to show his buddy.  He gets the URL
>in
>> his clipboard.  But his buddy is there he doesn't need it.
>
>And what about the piece of code that was cut into the clipboard just
>before the interruption?

The guy is clicking the link to get a highlight...

... which is a new feature introduced in my patches last week and not in mainline...

... so this wasn't the first time he clicked on line-number links in the Dark Timeline where this patch was accepted...

... so a little animation of a clipboard icon fading out would be enough to have taught him highlight = clipboard now.


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 2/6] ui-shared: line range highlight: introduce javascript
  2018-06-24 11:28     ` john
@ 2018-06-25  2:04       ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  2:04 UTC (permalink / raw)




On 06/24/2018 07:28 PM, John Keeping wrote:
> The subject is "ui-shared: ..." but should be "cgit.js" now I think.
> 
> On Sun, Jun 24, 2018 at 10:44:34AM +0800, Andy Green wrote:
>> diff --git a/cgit.js b/cgit.js
>> index e69de29..501c98f 100644
>> --- a/cgit.js
>> +++ b/cgit.js
>> @@ -0,0 +1,53 @@
>> +function cgit_line_range_highlight()
>> +{
>> +	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
>> +
>> +	l1 = parseInt(h.substring(2));
>> +	t = h.indexOf("-");
>> +	if (t >= 1)
>> +		l2 = parseInt(h.substring(t + 1));
>> +	else
>> +		l2 = l1;
>> +
>> +	if (!l1)
>> +		return;
>> +
>> +	var lh, t = 0, e1, e2, de;
>> +
>> +	e1 = e = document.getElementById('n' + l1);
>> +	if (!e)
>> +		return;
>> +
>> +	while (e1) { if (e1.offsetTop) t += e1.offsetTop; e1 = e1.offsetParent; }
>> +
>> +	de = document.createElement("DIV");
>> +
>> +	de.className = "selected-lines";
>> +	de.style.bottom = e.style.bottom;
>> +	de.style.top = t + 'px';
>> +
>> +	/* DOM structure is different by one level for /blame/ */
>> +	e1 = e.parentElement.parentElement.parentNode;
>> +	if (e1.offsetWidth < e1.parentNode.offsetWidth) /* blame */
>> +		e1 = e1.parentNode;
> 
> I guess this is trying to find the <tr> element, so why not look for
> that directly?
> 
> 	while (e1.tagName.toUpperCase() != 'TR')
> 		e1 = e1.parentNode;
> 
>> +	de.style.width = e1.offsetWidth + 'px';
>> +	de.style.left = e1.parentNode.parentNode.offsetLeft + 'px';
> 
> Again, if we're looking for the <table> here, can we make this more
> robust by looking for it explicitly?
> 
>> +	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
>> +
>> +	e.parentElement.parentElement.insertBefore(de, e.parentElement.parentElement.firstChild);
> 
> If I'm following correctly, e.parentElement.parentElement is either <td>
> in tree view or <div> in blame view.  Is that expected?
> 
> My thinking is that it is correct, but what we're looking for is the
> parent of the <pre> element containing the <a> with the line number ID.
> Some comments would really help make sense of these chains of parent
> references!

OK... I added a couple of comments but I think changing the var names to 
"etr" and "etable" and following the comment about looking for those 
specific names has obviated the rest of the need.

-Andy

>> +	while (l1 <= l2) {
>> +		e1 = document.getElementById('n' + l1++);
>> +		e1.style.backgroundColor = "yellow";
>> +	}
>> +
>> +	e.scrollIntoView(true);
>> +}
>> +
>> +/* line range highlight */
>> +
>> +document.addEventListener("DOMContentLoaded", function() {
>> +	cgit_line_range_highlight();
>> +}, false);


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v3 5/6] line-range-highlight: onclick handler and range selection
  2018-06-24 11:35     ` john
@ 2018-06-25  2:07       ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  2:07 UTC (permalink / raw)




On 06/24/2018 07:35 PM, John Keeping wrote:
> On Sun, Jun 24, 2018 at 10:44:49AM +0800, Andy Green wrote:
>> This allows the user to select line ranges simply by clicking on the
>> line number links.
>>
>>   - No selected highlit line, or a range already selected, causes the
>> click to highlight just the clicked line as usual.
>>
>>   - Clicking on a second line number link when a single line was already
>> highlit creates a line range highlight, between the lowest and
>> highest line numbers of the already-selected and newly-selected
>> line number links.
>>
>> The order of the clicks is unimportant, you can click the higher
>> line number link first and then the lower to define the range of
>> lines equally well.
>>
>> The implementation is slightly complicated by our single parent
>> onclick handler not being able to interrupt the already ongoing
>> processing of the a href #n change from the link click itself.
>>
>> Rather than bloat every linenumber link with its own onclick handler
>> defeating this default we simply do it with a single parent
>> onclick event and apply any computed range url in the existing
>> hashchange event handler.
>>
>> Signed-off-by: Andy Green <andy at warmcat.com>
>> ---
>>   cgit.js    |   39 +++++++++++++++++++++++++++++++++++++++
>>   ui-blame.c |    2 +-
>>   ui-tree.c  |    2 +-
>>   3 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/cgit.js b/cgit.js
>> index 29b2c3d..2cfad29 100644
>> --- a/cgit.js
>> +++ b/cgit.js
>> @@ -1,7 +1,15 @@
>> +var cgit_line_range_override;
>> +
>>   function cgit_line_range_highlight()
>>   {
>>   	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
>>   
>> +	if (cgit_line_range_override) {
>> +		window.location = cgit_line_range_override;
>> +		cgit_line_range_override = null;
>> +		return;
>> +	}
>> +
>>   	e = document.getElementById("cgit_line_range");
>>   	if (e) {
>>   		l1 = e.l1;
>> @@ -81,10 +89,41 @@ function cgit_line_range_highlight()
>>   		e.scrollIntoView(true);
>>   }
>>   
>> +function cgit_line_range_click(e) {
>> +	var t = e.target.id;
>> +
>> +	cgit_line_range_override = null;
>> +
>> +	/*
>> +	 * We are called while window location update from the
>> +	 * click on the #n link is underway.  So stash any
>> +	 * computed range #url for when the hashchange hander
>> +	 * is called, and override it there.
>> +	 */
>> +
>> +	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 =
>> +				window.location + '-' + t.substring(1);
>> +		else /* backwards */
>> +			cgit_line_range_override =
>> +				window.location.href.substring(0,
>> +					window.location.href.length -
>> +					window.location.hash.length) +
>> +				'#n' + t.substring(1) + '-' +
>> +					window.location.href.substring(
>> +						window.location.href.length -
>> +						window.location.hash.length + 2);
>> +}
>> +
>>   /* line range highlight */
>>   
>>   document.addEventListener("DOMContentLoaded", function() {
>>   	cgit_line_range_highlight();
>> +	var e = document.getElementById("linenumber_td");
>> +	if (e)
>> +		e.addEventListener("click", cgit_line_range_click, true);
>>   }, false);
>>   window.addEventListener("hashchange", function() {
>>   	cgit_line_range_highlight();
>> diff --git a/ui-blame.c b/ui-blame.c
>> index 50d0580..0ba8a5a 100644
>> --- a/ui-blame.c
>> +++ b/ui-blame.c
>> @@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
>>   
>>   	/* Line numbers */
>>   	if (ctx.cfg.enable_tree_linenumbers) {
>> -		html("<td class='linenumbers'>");
>> +		html("<td id=linenumber_td class='linenumbers'>");
> 
> We normally quote attribute values in all cases (like for "class" here),
> and I think we choose kebab-case over snake_case for HTML identifiers,
> by the same principle that we do so for CSS classes.
> 
> But can we just call this "linenumbers"?  I don't think we need the
> element type in the identifier.

OK.  The quotes are actually mandatory for well-formed html too.

-Andy

>>   		for (ent = sb.ent; ent; ent = ent->next) {
>>   			html("<div class='alt'><pre>");
>>   			emit_blame_entry_linenumber(ent);
>> diff --git a/ui-tree.c b/ui-tree.c
>> index e4cb558..6759b11 100644
>> --- a/ui-tree.c
>> +++ b/ui-tree.c
>> @@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
>>   	html("<table summary='blob content' class='blob'>\n");
>>   
>>   	if (ctx.cfg.enable_tree_linenumbers) {
>> -		html("<tr><td class='linenumbers'><pre>");
>> +		html("<tr><td id=linenumber_td class='linenumbers'><pre>");
>>   		idx = 0;
>>   		lineno = 0;
>>   


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 0/6] line range highlight
  2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
                   ` (4 preceding siblings ...)
  2018-06-24  2:44 ` [PATCH v3 0/6] line range highlight andy
@ 2018-06-25  5:49 ` andy
  2018-06-25  5:49   ` [PATCH v4 1/6] config: add js andy
                     ` (5 more replies)
  2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
  7 siblings, 6 replies; 73+ messages in thread
From: andy @ 2018-06-25  5:49 UTC (permalink / raw)


The following series adds the ability to direct the browser
to highlight specific lines and ranges of lines in
/tree/, /source/, and /blame/ views, using the existing
#URLs.

As part of the implementation it adds a new cgit.js file
that is included in cgit page <head> along with a new
config "js" to specify its url, defaulting to "/cgit.js".
It can be disabled by setting the "js" config to an empty
string.

Since v3 the JS had a cleanup from comments, and there is
now a clipboard icon (unrelated to clippy...) that appears
for 5s after clicking on a line number link to change the
highlight.  If you click on it, it copies the current highlight
#URL to the clipboard so you can share it.

You can find the patches as a usable tree here

https://warmcat.com/git/cgit/log/

Examples:

https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n152
https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n39-47

---

Andy Green (6):
      config: add js
      cgit.js: line range highlight: introduce javascript
      cgit.js: line range highlight: make responsive to url changes
      cgit.js: line range highlight: improve vertical scroll logic
      line-range-highlight: onclick handler and range selection
      line-range-highlight: copy URL to clipboard UI


 Makefile     |    1 
 cgit.c       |    3 +
 cgit.css     |   27 ++++++
 cgit.h       |    1 
 cgit.js      |  244 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 cgitrc.5.txt |    5 +
 ui-blame.c   |    2 
 ui-shared.c  |    5 +
 ui-tree.c    |    2 
 9 files changed, 288 insertions(+), 2 deletions(-)
 create mode 100644 cgit.js

--
Signature


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 1/6] config: add js
  2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
@ 2018-06-25  5:49   ` andy
  2018-06-26  8:03     ` list
  2018-06-25  5:49   ` [PATCH v4 2/6] cgit.js: line range highlight: introduce javascript andy
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-25  5:49 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Setting the js path to an empty string disables
emitting the reference to it in the head section.

Signed-off-by: Andy Green <andy at warmcat.com>
Reviewed-by: John Keeping <john at keeping.me.uk>
---
 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgit.js      |    0 
 cgitrc.5.txt |    5 +++++
 ui-shared.c  |    5 +++++
 6 files changed, 15 insertions(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 137150c..de7e13e 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 99ea7a2..e5a703e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..2737008 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,11 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".  Setting this to an empty string will
+	disable generation of the link to this file in the head section.
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".
diff --git a/ui-shared.c b/ui-shared.c
index 082a6f1..e2f3d55 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -787,6 +787,11 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	if (*ctx.cfg.js) {
+		html("<script type='text/javascript' src='");
+		html_attr(ctx.cfg.js);
+		html("'/></script>\n");
+	}
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 2/6] cgit.js: line range highlight: introduce javascript
  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-25  5:49   ` andy
  2018-06-25  5:49   ` [PATCH v4 3/6] cgit.js: line range highlight: make responsive to url changes andy
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  5:49 UTC (permalink / raw)


This adds a small css class, and a clientside js function plus
event registrations in cgit.js, to interpret the # part of the
URL on the client, and apply a highlight to filtered source.

Unlike blame highlight boxes which use generated divs, this
applies a computed absolutely-positioned, transparent div highlight
over the affected line(s) on the client side.

The # part of the URL is defined to not be passed to the server,
so the highlight can't be rendered on the server side.
However this has the advantage that the line range highlight
can operate on /blame/ urls trivially, since it doesn't
conflict with blame's generated div scheme.

pointer-events: none is used on the highlight overlay div to
allow the user to cut-and-paste in the highlit region and
click on links underneath normally.

The JS supports highlighting single lines as before like #n123
and also ranges of lines like #n123-135.

Because the browser can no longer automatically scroll to the
element in the second case, the JS also takes care of extracting
the range start element and scrolling to it dynamically.

Tested on Linux Firefox 60 + Linux Chrome 67

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |    7 +++++++
 cgit.js  |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/cgit.css b/cgit.css
index 61bbd49..443e04c 100644
--- a/cgit.css
+++ b/cgit.css
@@ -368,6 +368,13 @@ div#cgit table.blame td.lines > div > pre {
 	top: 0;
 }
 
+div#cgit div.selected-lines {
+	position: absolute;
+	pointer-events: none;
+	z-index: 1;
+	background-color: rgba(255, 255, 0, 0.2);
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index e69de29..2492b1b 100644
--- a/cgit.js
+++ b/cgit.js
@@ -0,0 +1,66 @@
+function cgit_line_range_highlight()
+{
+	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
+
+	l1 = parseInt(h.substring(2));
+	t = h.indexOf("-");
+	if (t >= 1)
+		l2 = parseInt(h.substring(t + 1));
+	else
+		l2 = l1;
+
+	if (!l1)
+		return;
+
+	if (l2 < l1)
+		l2 = l1;
+
+	var lh, e1, etable, etr, de, n;
+
+	t = 0;
+	e1 = e = document.getElementById('n' + l1);
+	if (!e)
+		return;
+
+	while (e1) {
+		if (e1.offsetTop)
+			t += e1.offsetTop;
+		e1 = e1.offsetParent;
+	}
+
+	de = document.createElement("DIV");
+
+	de.className = "selected-lines";
+	de.style.bottom = e.style.bottom;
+	de.style.top = t + 'px';
+
+	/* we will tack the highlight div at the parent tr */
+	etr = e;
+	while (etr.tagName.toLowerCase() != "tr")
+		etr = etr.parentNode;
+
+	de.style.width = etr.offsetWidth + 'px';
+
+	/* the table is offset from the left, the highlight
+	 * needs to follow it */
+	etable = etr;
+	while (etable.tagName.toLowerCase() != "table")
+		etable = etable.parentNode;
+
+	de.style.left = etable.offsetLeft + 'px';
+	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
+
+	etr.insertBefore(de, etr.firstChild);
+
+	n = l1;
+	while (n <= l2)
+		document.getElementById('n' + n++).style.backgroundColor = "yellow";
+
+	e.scrollIntoView(true);
+}
+
+/* line range highlight */
+
+document.addEventListener("DOMContentLoaded", function() {
+	cgit_line_range_highlight();
+}, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 3/6] cgit.js: line range highlight: make responsive to url changes
  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-25  5:49   ` [PATCH v4 2/6] cgit.js: line range highlight: introduce javascript andy
@ 2018-06-25  5:49   ` andy
  2018-06-25  5:50   ` [PATCH v4 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  5:49 UTC (permalink / raw)


Browser default interpretation of the # part of the URL is
to try to match it to an element ID one time at page load.

Subsequent modifications in the URL bar are ignored, even if
you hit enter there.

This patch makes the line range highlight able to clean up
after itself and be reapplied, and has the highlight function
listen for hashchange events and re-interpret the # part
when they occur.

Now if you edit the URL and press enter, any existing highlight
is removed, the # part reinterpreted and the new highlight
applied automatically.

This is particularly useful if you edit the # link with the
intention to show a range before copying the URL.

Clicking on the line number, which changes the URL bar to
have a corresponding #n part, also triggers hashchange and
makes the clicked line be highlit immediately.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/cgit.js b/cgit.js
index 2492b1b..9defdfd 100644
--- a/cgit.js
+++ b/cgit.js
@@ -2,8 +2,24 @@ function cgit_line_range_highlight()
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	e = document.getElementById("cgit_line_range");
+	if (e) {
+		l1 = e.l1;
+		while (l1 <= e.l2) {
+			var e1;
+			e1 = document.getElementById('n' + l1++);
+			e1.style.backgroundColor = null;
+		}
+
+		e.remove();
+	}
+
 	l1 = parseInt(h.substring(2));
+	if (!l1)
+		return;
+
 	t = h.indexOf("-");
+	l2 = l1;
 	if (t >= 1)
 		l2 = parseInt(h.substring(t + 1));
 	else
@@ -33,6 +49,9 @@ function cgit_line_range_highlight()
 	de.className = "selected-lines";
 	de.style.bottom = e.style.bottom;
 	de.style.top = t + 'px';
+	de.id = "cgit_line_range";
+	de.l1 = l1;
+	de.l2 = l2;
 
 	/* we will tack the highlight div at the parent tr */
 	etr = e;
@@ -64,3 +83,6 @@ function cgit_line_range_highlight()
 document.addEventListener("DOMContentLoaded", function() {
 	cgit_line_range_highlight();
 }, false);
+window.addEventListener("hashchange", function() {
+	cgit_line_range_highlight();
+}, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 4/6] cgit.js: line range highlight: improve vertical scroll logic
  2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
                     ` (2 preceding siblings ...)
  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   ` 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
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  5:50 UTC (permalink / raw)


Instead of following the browser heuristic to put any matching
id element to the top left of the browser window, compute the
number of visible lines vertically in the window, and the
middle of the highlit range, and try to centre the middle of
the highlit range in the window.

If the top of the range is no longer visible due to a range
consisting of more lines than the window can show, fall back to
placing the top of the range at the top of the window.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index 9defdfd..5941903 100644
--- a/cgit.js
+++ b/cgit.js
@@ -31,7 +31,7 @@ function cgit_line_range_highlight()
 	if (l2 < l1)
 		l2 = l1;
 
-	var lh, e1, etable, etr, de, n;
+	var lh, e1, etable, etr, de, n, hl, v;
 
 	t = 0;
 	e1 = e = document.getElementById('n' + l1);
@@ -75,7 +75,18 @@ function cgit_line_range_highlight()
 	while (n <= l2)
 		document.getElementById('n' + n++).style.backgroundColor = "yellow";
 
-	e.scrollIntoView(true);
+	hl = (window.innerHeight / (e.offsetHeight + 1));
+	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
+	if (v > l1)
+		v = l1;
+	if (v < 0)
+		v = 1;
+
+	e1 = document.getElementById('n' + Math.round(v));
+	if (e1)
+		e1.scrollIntoView(true);
+	else
+		e.scrollIntoView(true);
 }
 
 /* line range highlight */



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 5/6] line-range-highlight: onclick handler and range selection
  2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
                     ` (3 preceding siblings ...)
  2018-06-25  5:50   ` [PATCH v4 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
@ 2018-06-25  5:50   ` andy
  2018-06-25  5:50   ` [PATCH v4 6/6] line-range-highlight: copy URL to clipboard UI andy
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  5:50 UTC (permalink / raw)


This allows the user to select line ranges simply by clicking on the
line number links.

 - No selected highlit line, or a range already selected, causes the
click to highlight just the clicked line as usual.

 - Clicking on a second line number link when a single line was already
highlit creates a line range highlight, between the lowest and
highest line numbers of the already-selected and newly-selected
line number links.

The order of the clicks is unimportant, you can click the higher
line number link first and then the lower to define the range of
lines equally well.

The implementation is slightly complicated by our single parent
onclick handler not being able to interrupt the already ongoing
processing of the a href #n change from the link click itself.

Rather than bloat every linenumber link with its own onclick handler
defeating this default we simply do it with a single parent
onclick event and apply any computed range url in the existing
hashchange event handler.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js    |   42 ++++++++++++++++++++++++++++++++++++++++++
 ui-blame.c |    2 +-
 ui-tree.c  |    2 +-
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index 5941903..b766111 100644
--- a/cgit.js
+++ b/cgit.js
@@ -1,7 +1,15 @@
+var cgit_line_range_override;
+
 function cgit_line_range_highlight()
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	if (cgit_line_range_override) {
+		window.location = cgit_line_range_override;
+		cgit_line_range_override = null;
+		return;
+	}
+
 	e = document.getElementById("cgit_line_range");
 	if (e) {
 		l1 = e.l1;
@@ -89,10 +97,44 @@ function cgit_line_range_highlight()
 		e.scrollIntoView(true);
 }
 
+function cgit_line_range_click(e) {
+	var t = e.target.id,
+	    n = window.location.href.length - window.location.hash.length;
+
+	cgit_line_range_override = null;
+
+	/*
+	 * We are called while window location update from the
+	 * click on the #n link is underway.  So stash any
+	 * computed range #url for when the hashchange hander
+	 * is called, and override it there.
+	 */
+
+	if (!window.location.hash ||
+	    window.location.hash.indexOf("-") >= 0)
+		return;
+
+	if (parseInt(window.location.hash.substring(2)) <
+	    parseInt(t.substring(1))) { /* forwards */
+		cgit_line_range_override =
+			window.location + '-' + t.substring(1);
+
+		return;
+	}
+
+	cgit_line_range_override =
+		window.location.href.substring(0, n) +
+		'#n' + t.substring(1) + '-' +
+		window.location.href.substring(n + 2);
+}
+
 /* line range highlight */
 
 document.addEventListener("DOMContentLoaded", function() {
 	cgit_line_range_highlight();
+	var e = document.getElementById("linenumbers");
+	if (e)
+		e.addEventListener("click", cgit_line_range_click, true);
 }, false);
 window.addEventListener("hashchange", function() {
 	cgit_line_range_highlight();
diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..c9cca18 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	/* Line numbers */
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<td class='linenumbers'>");
+		html("<td id='linenumbers' class='linenumbers'>");
 		for (ent = sb.ent; ent; ent = ent->next) {
 			html("<div class='alt'><pre>");
 			emit_blame_entry_linenumber(ent);
diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..f96b845 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
 	html("<table summary='blob content' class='blob'>\n");
 
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<tr><td class='linenumbers'><pre>");
+		html("<tr><td id='linenumbers' class='linenumbers'><pre>");
 		idx = 0;
 		lineno = 0;
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 6/6] line-range-highlight: copy URL to clipboard UI
  2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
                     ` (4 preceding siblings ...)
  2018-06-25  5:50   ` [PATCH v4 5/6] line-range-highlight: onclick handler and range selection andy
@ 2018-06-25  5:50   ` andy
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-25  5:50 UTC (permalink / raw)


Clicking on the line numbers to control the highlight
now causes a clipboard icon to appear to the left of
the line for 5s.

Clicking this will copy the highlight URL to the
clipboard, and cause the icon to grow, blur and
fade to acknowledge the click.

After 5s the clipboard icon will fade out... you can
still collect the URL from the browser URL bar.

Clicking on other line  number links will remove any
clipboard icon and create a new one with a new 5s
lifetime.

This is an entirely clientside implementation in
cgit.css and cgit.js only.  The clipboard symbol is
from unicode.

Tested on Linux Chrome 67 + Firefox 60.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |   20 +++++++++
 cgit.js  |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 142 insertions(+), 19 deletions(-)

diff --git a/cgit.css b/cgit.css
index 443e04c..51f8691 100644
--- a/cgit.css
+++ b/cgit.css
@@ -375,6 +375,26 @@ div#cgit div.selected-lines {
 	background-color: rgba(255, 255, 0, 0.2);
 }
 
+div#cgit div.selected-lines-popup {
+	position: absolute;
+	text-align: center;
+	font-size: 28px;
+	font-color: black;
+	left: 8px;
+	width: 32px;
+	height: 32px;
+	opacity: 0;
+	z-index: 2;
+	cursor: pointer;
+	filter: grayscale(100%);
+	background-color: rgba(255, 255, 255, 0.0);
+	transition: filter 1s, opacity 1.5s, font-size 1s;
+}
+
+div#cgit div.selected-lines-popup:hover {
+	filter: grayscale(0%);
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index b766111..96921b3 100644
--- a/cgit.js
+++ b/cgit.js
@@ -1,4 +1,28 @@
-var cgit_line_range_override;
+var cgit_clipboard_popup_duration = 5000;
+
+var cgit_line_range_override, cgit_clipboard_url,
+    cgit_clipboard_popup, cgit_clipboard_popup_time;
+
+function cgit_collect_offsetTop(e1)
+{
+	var t = 0;
+
+	while (e1) {
+		if (e1.offsetTop)
+			t += e1.offsetTop;
+		e1 = e1.offsetParent;
+	}
+
+	return t;
+}
+
+function cgit_find_parent_of_type(e, type)
+{
+	while (e.tagName.toLowerCase() != type)
+		e = e.parentNode;
+
+	return e;
+}
 
 function cgit_line_range_highlight()
 {
@@ -41,38 +65,27 @@ function cgit_line_range_highlight()
 
 	var lh, e1, etable, etr, de, n, hl, v;
 
-	t = 0;
 	e1 = e = document.getElementById('n' + l1);
 	if (!e)
 		return;
 
-	while (e1) {
-		if (e1.offsetTop)
-			t += e1.offsetTop;
-		e1 = e1.offsetParent;
-	}
-
 	de = document.createElement("DIV");
 
 	de.className = "selected-lines";
 	de.style.bottom = e.style.bottom;
-	de.style.top = t + 'px';
+	de.style.top = cgit_collect_offsetTop(e1) + 'px';
 	de.id = "cgit_line_range";
 	de.l1 = l1;
 	de.l2 = l2;
 
 	/* we will tack the highlight div at the parent tr */
-	etr = e;
-	while (etr.tagName.toLowerCase() != "tr")
-		etr = etr.parentNode;
+	etr = cgit_find_parent_of_type(e, "tr");
 
 	de.style.width = etr.offsetWidth + 'px';
 
 	/* the table is offset from the left, the highlight
 	 * needs to follow it */
-	etable = etr;
-	while (etable.tagName.toLowerCase() != "table")
-		etable = etable.parentNode;
+	etable = cgit_find_parent_of_type(etr, "table");
 
 	de.style.left = etable.offsetLeft + 'px';
 	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
@@ -97,11 +110,97 @@ function cgit_line_range_highlight()
 		e.scrollIntoView(true);
 }
 
+function cgit_copy_clipboard(value)
+{
+	var inp = document.createElement("textarea");
+	var e = document.getElementById("linenumbers");
+
+	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_popup_click(e) {
+	e.stopPropagation();
+	e.preventDefault();
+	cgit_copy_clipboard(cgit_clipboard_url);
+	/* fade out, blur out, grow */
+	cgit_clipboard_popup.style.opacity = "0";
+	cgit_clipboard_popup.style.fontSize = "40px";
+	cgit_clipboard_popup.style.filter = "blur(2px)";
+}
+
+function cgit_line_popup_create(e)
+{
+	var e1 = e, etable, d = new Date;
+
+	if (cgit_clipboard_popup)
+		cgit_clipboard_popup.remove();
+
+	console.log(e.offsetHeight);
+
+	cgit_clipboard_popup = document.createElement("DIV");
+	cgit_clipboard_popup.className = "selected-lines-popup";
+
+	cgit_clipboard_popup.style.top = cgit_collect_offsetTop(e1) + "px";
+	cgit_clipboard_popup.innerHTML = "&#x1f4cb;";
+	/* event listener cannot override default browser #URL behaviour */
+	cgit_clipboard_popup.onclick = cgit_line_popup_click;
+
+	etable = cgit_find_parent_of_type(e, "table");
+	etable.insertBefore(cgit_clipboard_popup, etable.firstChild);
+	cgit_clipboard_popup_time = d.getTime();
+
+	setTimeout(function() {
+		cgit_clipboard_popup.style.opacity = "1";
+	}, 1);
+
+	setTimeout(function() {
+		var d = new Date;
+		if (!cgit_clipboard_popup)
+			return;
+		if (d.getTime() - cgit_clipboard_popup_time < cgit_clipboard_popup_duration)
+			return;
+		cgit_clipboard_popup.style.opacity = "0";
+		setTimeout(function() {
+			var d = new Date;
+			if (!cgit_clipboard_popup)
+				return;
+			if (d.getTime() - cgit_clipboard_popup_time < cgit_clipboard_popup_duration + 1000)
+				return;
+
+			cgit_clipboard_popup.remove();
+			cgit_clipboard_popup = null;
+		}, 1000);
+	}, cgit_clipboard_popup_duration);
+
+}
+
 function cgit_line_range_click(e) {
-	var t = e.target.id,
+	var t = e.target.id, elem,
 	    n = window.location.href.length - window.location.hash.length;
 
+	if (!t)
+		return;
+
+	elem = document.getElementById(t);
+
+	if (!elem)
+		return;
+
 	cgit_line_range_override = null;
+	cgit_line_popup_create(elem);
 
 	/*
 	 * We are called while window location update from the
@@ -111,18 +210,22 @@ function cgit_line_range_click(e) {
 	 */
 
 	if (!window.location.hash ||
-	    window.location.hash.indexOf("-") >= 0)
+	    window.location.hash.indexOf("-") >= 0) {
+		cgit_clipboard_url = window.location.href.substring(0, n) +
+				     '#n' + t.substring(1);
+
 		return;
+	}
 
 	if (parseInt(window.location.hash.substring(2)) <
 	    parseInt(t.substring(1))) { /* forwards */
-		cgit_line_range_override =
+		cgit_clipboard_url = cgit_line_range_override =
 			window.location + '-' + t.substring(1);
 
 		return;
 	}
 
-	cgit_line_range_override =
+	cgit_clipboard_url = cgit_line_range_override =
 		window.location.href.substring(0, n) +
 		'#n' + t.substring(1) + '-' +
 		window.location.href.substring(n + 2);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v4 1/6] config: add js
  2018-06-25  5:49   ` [PATCH v4 1/6] config: add js andy
@ 2018-06-26  8:03     ` list
  0 siblings, 0 replies; 73+ messages in thread
From: list @ 2018-06-26  8:03 UTC (permalink / raw)


Andy Green <andy at warmcat.com> on Mon, 2018/06/25 13:49:
> Just like the config allows setting css URL path,
> add a config for setting the js URL path
> 
> Setting the js path to an empty string disables
> emitting the reference to it in the head section.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
>  Makefile     |    1 +
>  cgit.c       |    3 +++
>  cgit.h       |    1 +
>  cgit.js      |    0 
>  cgitrc.5.txt |    5 +++++
>  ui-shared.c  |    5 +++++
>  6 files changed, 15 insertions(+)
>  create mode 100644 cgit.js
> 
> diff --git a/Makefile b/Makefile
> index 137150c..de7e13e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -87,6 +87,7 @@ install: all
>  	$(INSTALL) -m 0755 cgit
> $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME) $(INSTALL) -m 0755 -d
> $(DESTDIR)$(CGIT_DATA_PATH) $(INSTALL) -m 0644 cgit.css
> $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
> +	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
>  	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
>  	$(INSTALL) -m 0644 favicon.ico
> $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico $(INSTALL) -m 0644 robots.txt
> $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt diff --git a/cgit.c b/cgit.c
> index bdb2fad..8b23c8f 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -146,6 +146,8 @@ static void config_cb(const char *name, const char
> *value) ctx.cfg.root_readme = xstrdup(value);
>  	else if (!strcmp(name, "css"))
>  		ctx.cfg.css = xstrdup(value);
> +	else if (!strcmp(name, "js"))
> +		ctx.cfg.js = xstrdup(value);
>  	else if (!strcmp(name, "favicon"))
>  		ctx.cfg.favicon = xstrdup(value);
>  	else if (!strcmp(name, "footer"))
> @@ -384,6 +386,7 @@ static void prepare_context(void)
>  	ctx.cfg.branch_sort = 0;
>  	ctx.cfg.commit_sort = 0;
>  	ctx.cfg.css = "/cgit.css";
> +	ctx.cfg.js = "/cgit.js";
>  	ctx.cfg.logo = "/cgit.png";
>  	ctx.cfg.favicon = "/favicon.ico";
>  	ctx.cfg.local_time = 0;
> diff --git a/cgit.h b/cgit.h
> index 99ea7a2..e5a703e 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -194,6 +194,7 @@ struct cgit_config {
>  	char *clone_prefix;
>  	char *clone_url;
>  	char *css;
> +	char *js;
>  	char *favicon;
>  	char *footer;
>  	char *head_include;
> diff --git a/cgit.js b/cgit.js
> new file mode 100644
> index 0000000..e69de29
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 99fc799..2737008 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -248,6 +248,11 @@ inline-readme::
>  	individually also choose to ignore this global list, and create a
>  	repo-specific list by using 'repo.inline-readme'.
>  
> +js::
> +	Url which specifies the javascript script document to include in
> all cgit
> +	pages.  Default value: "/cgit.js".  Setting this to an empty
> string will
> +	disable generation of the link to this file in the head section.
> +
>  local-time::
>  	Flag which, if set to "1", makes cgit print commit and tag times
> in the servers timezone. Default value: "0".
> diff --git a/ui-shared.c b/ui-shared.c
> index 082a6f1..e2f3d55 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -787,6 +787,11 @@ void cgit_print_docstart(void)
>  	html("<link rel='stylesheet' type='text/css' href='");
>  	html_attr(ctx.cfg.css);
>  	html("'/>\n");  
> +	if (*ctx.cfg.js) {
> +		html("<script type='text/javascript' src='");
> +		html_attr(ctx.cfg.js);
> +		html("'/></script>\n");

The opening script tag should not have a closing slash when a closing tag
follows.

> +	}
>  	if (ctx.cfg.favicon) {
>  		html("<link rel='shortcut icon' href='");
>  		html_attr(ctx.cfg.favicon);

-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20180626/737ca7d3/attachment.asc>


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 0/6] line range highlight
  2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
                   ` (5 preceding siblings ...)
  2018-06-25  5:49 ` [PATCH v4 0/6] line range highlight andy
@ 2018-06-26 11:25 ` andy
  2018-06-26 11:25   ` [PATCH v5 1/6] config: add js andy
                     ` (5 more replies)
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
  7 siblings, 6 replies; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


The following series adds the ability to direct the browser
to highlight specific lines and ranges of lines in
/tree/, /source/, and /blame/ views, using the existing
#URLs.

As part of the implementation it adds a new cgit.js file
that is included in cgit page <head> along with a new
config "js" to specify its url, defaulting to "/cgit.js".
It can be disabled by setting the "js" config to an empty
string.

Highlit lines or ranges are centred vertically rather than
the default of scrolled to the top of the browser window
area.

You can click on the line number links to highlight
individual lines, or line ranges when an individual line
is already highlit.  If a range is highlit, clicking on
a line number link clears the range and just highlights
that line again.

When you click on a line number link, a clipboard icon
appears for 5s on the left of the link.  If you click on
this icon, the current #URL is copied to the clipboard.

CSS transitions are used throughout.

v5 cleans the js up a bit and disables the browser
response to clicking on a #URL, stopping the scroll position
jumping around.

It also switches to listening for window.load to apply the
initial highlight.  Because wait on the DOM load is racy
for the layout having access to header image size... the
page vertical position can change after DOM load is complete.

You can find the patches as a usable tree here

https://warmcat.com/git/cgit/log/

Examples:

https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n152
https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n39-47
---

Andy Green (6):
      config: add js
      cgit.js: line range highlight: introduce javascript
      cgit.js: line range highlight: make responsive to url changes
      cgit.js: line range highlight: improve vertical scroll logic
      line-range-highlight: onclick handler and range selection
      line-range-highlight: copy URL to clipboard UI


 Makefile     |    1 
 cgit.c       |    3 +
 cgit.css     |   29 +++++++
 cgit.h       |    1 
 cgit.js      |  225 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 cgitrc.5.txt |    5 +
 ui-blame.c   |    2 -
 ui-shared.c  |    5 +
 ui-tree.c    |    2 -
 9 files changed, 271 insertions(+), 2 deletions(-)
 create mode 100644 cgit.js

--
Signature


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 1/6] config: add js
  2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
@ 2018-06-26 11:25   ` andy
  2018-06-26 11:25   ` [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript andy
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Setting the js path to an empty string disables
emitting the reference to it in the head section.

Signed-off-by: Andy Green <andy at warmcat.com>
Reviewed-by: John Keeping <john at keeping.me.uk>
---
 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgit.js      |    0 
 cgitrc.5.txt |    5 +++++
 ui-shared.c  |    5 +++++
 6 files changed, 15 insertions(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 137150c..de7e13e 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 999db9e..582416e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..2737008 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,11 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".  Setting this to an empty string will
+	disable generation of the link to this file in the head section.
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".
diff --git a/ui-shared.c b/ui-shared.c
index 0fe4cbf..ff2a3e1 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -792,6 +792,11 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	if (*ctx.cfg.js) {
+		html("<script type='text/javascript' src='");
+		html_attr(ctx.cfg.js);
+		html("'/></script>\n");
+	}
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript
  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   ` andy
  2018-06-27 18:02     ` Jason
  2018-06-26 11:25   ` [PATCH v5 3/6] cgit.js: line range highlight: make responsive to url changes andy
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


This adds a small css class, and a clientside js function plus
event registrations in cgit.js, to interpret the # part of the
URL on the client, and apply a highlight to filtered source.

Unlike blame highlight boxes which use generated divs, this
applies a computed absolutely-positioned, transparent div highlight
over the affected line(s) on the client side.

The # part of the URL is defined to not be passed to the server,
so the highlight can't be rendered on the server side.
However this has the advantage that the line range highlight
can operate on /blame/ urls trivially, since it doesn't
conflict with blame's generated div scheme.

pointer-events: none is used on the highlight overlay div to
allow the user to cut-and-paste in the highlit region and
click on links underneath normally.

The JS supports highlighting single lines as before like #n123
and also ranges of lines like #n123-135.

Because the browser can no longer automatically scroll to the
element in the second case, the JS also takes care of extracting
the range start element and scrolling to it dynamically.

Tested on Linux Firefox 60 + Linux Chrome 67

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |    9 +++++++
 cgit.js  |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/cgit.css b/cgit.css
index 61bbd49..13c0955 100644
--- a/cgit.css
+++ b/cgit.css
@@ -328,6 +328,7 @@ div#cgit table.ssdiff td.lineno a {
 	color: gray;
 	text-align: right;
 	text-decoration: none;
+	transition: background-color 1.5s;
 }
 
 div#cgit table.blob td.linenumbers a:hover,
@@ -368,6 +369,14 @@ div#cgit table.blame td.lines > div > pre {
 	top: 0;
 }
 
+div#cgit div.selected-lines {
+	position: absolute;
+	pointer-events: none;
+	z-index: 1;
+	background-color: rgba(255, 255, 0, 0);
+	transition: background-color 1.5s;
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index e69de29..bcaf875 100644
--- a/cgit.js
+++ b/cgit.js
@@ -0,0 +1,80 @@
+function cgit_collect_offsetTop(e1)
+{
+	var t = 0;
+
+	while (e1) {
+		if (e1.offsetTop)
+			t += e1.offsetTop;
+		e1 = e1.offsetParent;
+	}
+
+	return t;
+}
+
+function cgit_find_parent_of_type(e, type)
+{
+	while (e.tagName.toLowerCase() != type)
+		e = e.parentNode;
+
+	return e;
+}
+
+function cgit_line_range_highlight()
+{
+	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
+
+	l1 = parseInt(h.substring(2));
+	if (!l1)
+		return;
+
+	t = h.indexOf("-");
+	l2 = l1;
+	if (t >= 1)
+		l2 = parseInt(h.substring(t + 1));
+
+	if (l2 < l1)
+		l2 = l1;
+
+	var lh, etable, etr, de, n;
+
+	e = document.getElementById('n' + l1);
+	if (!e)
+		return;
+
+	de = document.createElement("DIV");
+
+	de.className = "selected-lines";
+	de.style.bottom = e.style.bottom;
+	de.style.top = cgit_collect_offsetTop(e) + 'px';
+	de.l1 = l1;
+	de.l2 = l2;
+
+	/* we will tack the highlight div at the parent tr */
+	etr = cgit_find_parent_of_type(e, "tr");
+
+	de.style.width = etr.offsetWidth + 'px';
+
+	/* the table is offset from the left, the highlight
+	 * needs to follow it */
+	etable = cgit_find_parent_of_type(etr, "table");
+
+	de.style.left = etable.offsetLeft + 'px';
+	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
+
+	etr.insertBefore(de, etr.firstChild);
+
+	setTimeout(function() {
+		de.style.backgroundColor = "rgba(255, 255, 0, 0.2)";
+	}, 1);
+
+	n = l1;
+	while (n <= l2)
+		document.getElementById('n' + n++).style.backgroundColor = "yellow";
+
+	e.scrollIntoView(true);
+}
+
+/* we have to use load, because header images can push the layout vertically */
+window.addEventListener("load", function() {
+	cgit_line_range_highlight();
+}, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 3/6] cgit.js: line range highlight: make responsive to url changes
  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-26 11:25   ` andy
  2018-06-26 11:25   ` [PATCH v5 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


Browser default interpretation of the # part of the URL is
to try to match it to an element ID one time at page load.

Subsequent modifications in the URL bar are ignored, even if
you hit enter there.

This patch makes the line range highlight able to clean up
after itself and be reapplied, and has the highlight function
listen for hashchange events and re-interpret the # part
when they occur.

Now if you edit the URL and press enter, any existing highlight
is removed, the # part reinterpreted and the new highlight
applied automatically.

This is particularly useful if you edit the # link with the
intention to show a range before copying the URL.

Clicking on the line number, which changes the URL bar to
have a corresponding #n part, also triggers hashchange and
makes the clicked line be highlit immediately.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/cgit.js b/cgit.js
index bcaf875..ce501bf 100644
--- a/cgit.js
+++ b/cgit.js
@@ -23,6 +23,18 @@ function cgit_line_range_highlight()
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	e = document.getElementById("cgit-line-range");
+	if (e) {
+		l1 = e.l1;
+		while (l1 <= e.l2) {
+			var e1;
+			e1 = document.getElementById('n' + l1++);
+			e1.style.backgroundColor = null;
+		}
+
+		e.remove();
+	}
+
 	l1 = parseInt(h.substring(2));
 	if (!l1)
 		return;
@@ -46,6 +58,7 @@ function cgit_line_range_highlight()
 	de.className = "selected-lines";
 	de.style.bottom = e.style.bottom;
 	de.style.top = cgit_collect_offsetTop(e) + 'px';
+	de.id = "cgit-line-range";
 	de.l1 = l1;
 	de.l2 = l2;
 
@@ -78,3 +91,6 @@ function cgit_line_range_highlight()
 window.addEventListener("load", function() {
 	cgit_line_range_highlight();
 }, false);
+window.addEventListener("hashchange", function() {
+	cgit_line_range_highlight();
+}, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 4/6] cgit.js: line range highlight: improve vertical scroll logic
  2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
                     ` (2 preceding siblings ...)
  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   ` andy
  2018-06-26 11:25   ` [PATCH v5 5/6] line-range-highlight: onclick handler and range selection andy
  2018-06-26 11:25   ` [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI andy
  5 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


Instead of following the browser heuristic to put any matching
id element to the top left of the browser window, compute the
number of visible lines vertically in the window, and the
middle of the highlit range, and try to centre the middle of
the highlit range in the window.

If the top of the range is no longer visible due to a range
consisting of more lines than the window can show, fall back to
placing the top of the range at the top of the window.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index ce501bf..114b0c5 100644
--- a/cgit.js
+++ b/cgit.js
@@ -47,7 +47,7 @@ function cgit_line_range_highlight()
 	if (l2 < l1)
 		l2 = l1;
 
-	var lh, etable, etr, de, n;
+	var lh, etable, etr, de, n, hl, v;
 
 	e = document.getElementById('n' + l1);
 	if (!e)
@@ -84,13 +84,25 @@ function cgit_line_range_highlight()
 	while (n <= l2)
 		document.getElementById('n' + n++).style.backgroundColor = "yellow";
 
-	e.scrollIntoView(true);
+	hl = (window.innerHeight / (e.offsetHeight + 1));
+	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
+	if (v > l1)
+		v = l1;
+	if (v < 1)
+		v = 1;
+
+	t = document.getElementById('n' + Math.round(v));
+	if (!t)
+		t = e;
+
+	t.scrollIntoView(true);
 }
 
 /* we have to use load, because header images can push the layout vertically */
 window.addEventListener("load", function() {
 	cgit_line_range_highlight();
 }, false);
+
 window.addEventListener("hashchange", function() {
 	cgit_line_range_highlight();
 }, false);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 5/6] line-range-highlight: onclick handler and range selection
  2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
                     ` (3 preceding siblings ...)
  2018-06-26 11:25   ` [PATCH v5 4/6] cgit.js: line range highlight: improve vertical scroll logic andy
@ 2018-06-26 11:25   ` 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
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


This allows the user to select line ranges simply by clicking on the
line number links.

 - No selected highlit line, or a range already selected, causes the
click to highlight just the clicked line as usual.

 - Clicking on a second line number link when a single line was already
highlit creates a line range highlight, between the lowest and
highest line numbers of the already-selected and newly-selected
line number links.

The order of the clicks is unimportant, you can click the higher
line number link first and then the lower to define the range of
lines equally well.

The implementation is slightly complicated by our single parent
onclick handler not being able to interrupt the already ongoing
processing of the a href #n change from the link click itself.

Rather than bloat every linenumber link with its own onclick handler
defeating this default we simply do it with a single parent
onclick event and apply any computed range url in the existing
hashchange event handler.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js    |   32 ++++++++++++++++++++++++++++++++
 ui-blame.c |    2 +-
 ui-tree.c  |    2 +-
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index 114b0c5..ebc4e5b 100644
--- a/cgit.js
+++ b/cgit.js
@@ -98,11 +98,43 @@ function cgit_line_range_highlight()
 	t.scrollIntoView(true);
 }
 
+function cgit_line_range_click(e) {
+	var t, m, n = window.location.href.length - window.location.hash.length;
+
+	/* disable passthru to stop needless scrolling by default browser #URL handler */
+	e.stopPropagation();
+	e.preventDefault();
+
+	if (!window.location.hash ||
+	    window.location.hash.indexOf("-") >= 0)
+		t = window.location.href.substring(0, n) +
+		    '#n' + e.target.id.substring(1);
+	else {
+		if (parseInt(window.location.hash.substring(2)) <
+		    parseInt(e.target.id.substring(1)))  /* forwards */
+			t = window.location + '-' + e.target.id.substring(1);
+		else
+			t = window.location.href.substring(0, n) +
+				'#n' + e.target.id.substring(1) + '-' +
+				window.location.href.substring(n + 2);
+	}
+
+	window.history.replaceState(null, null, t);
+
+	cgit_line_range_highlight();
+}
+
 /* we have to use load, because header images can push the layout vertically */
 window.addEventListener("load", function() {
 	cgit_line_range_highlight();
 }, false);
 
+document.addEventListener("DOMContentLoaded", function() {
+	/* event listener cannot override default #URL browser processing,
+	 * requires onclick */
+	document.getElementById("linenumbers").onclick = cgit_line_range_click;
+}, false);
+
 window.addEventListener("hashchange", function() {
 	cgit_line_range_highlight();
 }, false);
diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..c9cca18 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	/* Line numbers */
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<td class='linenumbers'>");
+		html("<td id='linenumbers' class='linenumbers'>");
 		for (ent = sb.ent; ent; ent = ent->next) {
 			html("<div class='alt'><pre>");
 			emit_blame_entry_linenumber(ent);
diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..f96b845 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
 	html("<table summary='blob content' class='blob'>\n");
 
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<tr><td class='linenumbers'><pre>");
+		html("<tr><td id='linenumbers' class='linenumbers'><pre>");
 		idx = 0;
 		lineno = 0;
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI
  2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
                     ` (4 preceding siblings ...)
  2018-06-26 11:25   ` [PATCH v5 5/6] line-range-highlight: onclick handler and range selection andy
@ 2018-06-26 11:25   ` andy
  2018-06-27 18:07     ` Jason
  5 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-26 11:25 UTC (permalink / raw)


Clicking on the line numbers to control the highlight
now causes a clipboard icon to appear to the left of
the line for 5s.

Clicking this will copy the highlight URL to the
clipboard, and cause the icon to grow, blur and
fade to acknowledge the click.

After 5s the clipboard icon will fade out... you can
still collect the URL from the browser URL bar.

Clicking on other line  number links will remove any
clipboard icon and create a new one with a new 5s
lifetime.

This is an entirely clientside implementation in
cgit.css and cgit.js only.  The clipboard symbol is
from unicode.

Tested on Linux Chrome 67 + Firefox 60.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |   20 ++++++++++++++
 cgit.js  |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/cgit.css b/cgit.css
index 13c0955..7c2bafe 100644
--- a/cgit.css
+++ b/cgit.css
@@ -377,6 +377,26 @@ div#cgit div.selected-lines {
 	transition: background-color 1.5s;
 }
 
+div#cgit div.selected-lines-popup {
+	position: absolute;
+	text-align: center;
+	font-size: 28px;
+	font-color: black;
+	left: 8px;
+	width: 32px;
+	height: 32px;
+	opacity: 0;
+	z-index: 2;
+	cursor: pointer;
+	filter: grayscale(100%);
+	background-color: rgba(255, 255, 255, 0.0);
+	transition: filter 1s, opacity 1.5s, font-size 1s;
+}
+
+div#cgit div.selected-lines-popup:hover {
+	filter: grayscale(0%);
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index ebc4e5b..cd7ea86 100644
--- a/cgit.js
+++ b/cgit.js
@@ -1,3 +1,7 @@
+var cgit_clipboard_popup_duration = 5000;
+
+var cgit_clipboard_popup, cgit_clipboard_popup_time;
+
 function cgit_collect_offsetTop(e1)
 {
 	var t = 0;
@@ -98,13 +102,94 @@ function cgit_line_range_highlight()
 	t.scrollIntoView(true);
 }
 
+function cgit_copy_clipboard(value)
+{
+	var inp = document.createElement("textarea");
+	var e = document.getElementById("linenumbers");
+
+	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_popup_click(e) {
+	e.stopPropagation();
+	e.preventDefault();
+	cgit_copy_clipboard(window.location);
+	/* fade out, blur out, grow */
+	cgit_clipboard_popup.style.opacity = "0";
+	cgit_clipboard_popup.style.fontSize = "40px";
+	cgit_clipboard_popup.style.filter = "blur(2px)";
+}
+
+function cgit_line_popup_create(e)
+{
+	var e1 = e, etable, d = new Date;
+
+	if (cgit_clipboard_popup)
+		cgit_clipboard_popup.remove();
+
+	cgit_clipboard_popup = document.createElement("DIV");
+	cgit_clipboard_popup.className = "selected-lines-popup";
+
+	cgit_clipboard_popup.style.top = cgit_collect_offsetTop(e1) + "px";
+	cgit_clipboard_popup.innerHTML = "&#x1f4cb;";
+	/* event listener cannot override default browser #URL behaviour */
+	cgit_clipboard_popup.onclick = cgit_line_popup_click;
+
+	etable = cgit_find_parent_of_type(e, "table");
+	etable.insertBefore(cgit_clipboard_popup, etable.firstChild);
+	cgit_clipboard_popup_time = d.getTime();
+
+	setTimeout(function() {
+		cgit_clipboard_popup.style.opacity = "1";
+	}, 1);
+
+	setTimeout(function() {
+		var d = new Date;
+		if (!cgit_clipboard_popup)
+			return;
+		if (d.getTime() - cgit_clipboard_popup_time < cgit_clipboard_popup_duration)
+			return;
+		cgit_clipboard_popup.style.opacity = "0";
+		setTimeout(function() {
+			var d = new Date;
+			if (!cgit_clipboard_popup)
+				return;
+			if (d.getTime() - cgit_clipboard_popup_time < cgit_clipboard_popup_duration + 1000)
+				return;
+
+			cgit_clipboard_popup.remove();
+			cgit_clipboard_popup = null;
+		}, 1000);
+	}, cgit_clipboard_popup_duration);
+
+}
+
 function cgit_line_range_click(e) {
-	var t, m, n = window.location.href.length - window.location.hash.length;
+	var t, elem, m, n = window.location.href.length - window.location.hash.length;
 
 	/* disable passthru to stop needless scrolling by default browser #URL handler */
 	e.stopPropagation();
 	e.preventDefault();
 
+	elem = document.getElementById(e.target.id);
+	if (!elem)
+		return;
+
+	cgit_line_popup_create(elem);
+
 	if (!window.location.hash ||
 	    window.location.hash.indexOf("-") >= 0)
 		t = window.location.href.substring(0, n) +



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5-ninjaedit] line-range-highlight: onclick handler and range selection
  2018-06-26 11:25   ` [PATCH v5 5/6] line-range-highlight: onclick handler and range selection andy
@ 2018-06-26 11:51     ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-26 11:51 UTC (permalink / raw)


This allows the user to select line ranges simply by clicking on the
line number links.

 - No selected highlit line, or a range already selected, causes the
click to highlight just the clicked line as usual.

 - Clicking on a second line number link when a single line was already
highlit creates a line range highlight, between the lowest and
highest line numbers of the already-selected and newly-selected
line number links.

The order of the clicks is unimportant, you can click the higher
line number link first and then the lower to define the range of
lines equally well.

The implementation is slightly complicated by our single parent
onclick handler not being able to interrupt the already ongoing
processing of the a href #n change from the link click itself.

Rather than bloat every linenumber link with its own onclick handler
defeating this default we simply do it with a single parent
onclick event and apply any computed range url in the existing
hashchange event handler.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js    |   34 ++++++++++++++++++++++++++++++++++
 ui-blame.c |    2 +-
 ui-tree.c  |    2 +-
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index 114b0c5..3018829 100644
--- a/cgit.js
+++ b/cgit.js
@@ -98,11 +98,45 @@ function cgit_line_range_highlight()
 	t.scrollIntoView(true);
 }
 
+function cgit_line_range_click(e) {
+	var t, m, n = window.location.href.length - window.location.hash.length;
+
+	/* disable passthru to stop needless scrolling by default browser #URL handler */
+	e.stopPropagation();
+	e.preventDefault();
+
+	if (!window.location.hash ||
+	    window.location.hash.indexOf("-") >= 0)
+		t = window.location.href.substring(0, n) +
+		    '#n' + e.target.id.substring(1);
+	else {
+		if (parseInt(window.location.hash.substring(2)) <
+		    parseInt(e.target.id.substring(1)))  /* forwards */
+			t = window.location + '-' + e.target.id.substring(1);
+		else
+			t = window.location.href.substring(0, n) +
+				'#n' + e.target.id.substring(1) + '-' +
+				window.location.href.substring(n + 2);
+	}
+
+	window.history.replaceState(null, null, t);
+
+	cgit_line_range_highlight();
+}
+
 /* we have to use load, because header images can push the layout vertically */
 window.addEventListener("load", function() {
 	cgit_line_range_highlight();
 }, false);
 
+document.addEventListener("DOMContentLoaded", function() {
+	/* event listener cannot override default #URL browser processing,
+	 * requires onclick */
+	var e = document.getElementById("linenumbers");
+	if (e)
+		e.onclick = cgit_line_range_click;
+}, false);
+
 window.addEventListener("hashchange", function() {
 	cgit_line_range_highlight();
 }, false);
diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..c9cca18 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	/* Line numbers */
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<td class='linenumbers'>");
+		html("<td id='linenumbers' class='linenumbers'>");
 		for (ent = sb.ent; ent; ent = ent->next) {
 			html("<div class='alt'><pre>");
 			emit_blame_entry_linenumber(ent);
diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..f96b845 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
 	html("<table summary='blob content' class='blob'>\n");
 
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<tr><td class='linenumbers'><pre>");
+		html("<tr><td id='linenumbers' class='linenumbers'><pre>");
 		idx = 0;
 		lineno = 0;
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript
  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
  0 siblings, 1 reply; 73+ messages in thread
From: Jason @ 2018-06-27 18:02 UTC (permalink / raw)


Hi Andy,

I'm super hesitant about the Pandora's box that introducing javascript
implies, but perhaps there's no use in fighting the future.

A few notes:

- Your js needs the copyright line like the rest of the project.
- Rather than awkwardly namespacing global methods, wrap everything
inside a big anonymous function.
- 1.5s is way too long for an animation. In fact, I'm not sure what an
animation communicates at all, and perhaps we could get rid of it.
- Setting colors from inside js is a big no-no. Instead, add and
remove classes from elements, and leave the styling to css.

Also, can this be done from pure css? For example, certainly
highlighting one line in pure css based on the #anchor is possible,
via css link selectors.

Jason


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI
  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
  0 siblings, 1 reply; 73+ messages in thread
From: Jason @ 2018-06-27 18:07 UTC (permalink / raw)


Nack. If we're going to add something like this, the github GUI for it
is clearly superior and the right way of going about things, with the
elegant [...] menu.

Jason


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript
  2018-06-27 18:02     ` Jason
@ 2018-06-27 21:45       ` andy
  2018-06-28 23:58         ` [PATCH] cgit.css: add copyright lines andy
  0 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-27 21:45 UTC (permalink / raw)




On 06/28/2018 02:02 AM, Jason A. Donenfeld wrote:
> Hi Andy,
> 
> I'm super hesitant about the Pandora's box that introducing javascript
> implies, but perhaps there's no use in fighting the future.

The "fighting the future" bus left the station ten years ago when github 
was founded.  Worrying about JS is just fighting the past.

> A few notes:
> 
> - Your js needs the copyright line like the rest of the project.
> - Rather than awkwardly namespacing global methods, wrap everything
> inside a big anonymous function.

OK.

> - 1.5s is way too long for an animation. In fact, I'm not sure what an
> animation communicates at all, and perhaps we could get rid of it.

We can't apply the highlight until the page completes its load event, 
because the vertical layout is subject to be adjusted by image loads etc 
right up until then.  It looked nicer to me to have the highlight 
"become apparent" in the case the lines had already been visibly 
rendered without it.

> - Setting colors from inside js is a big no-no. Instead, add and
> remove classes from elements, and leave the styling to css.

OK.

> Also, can this be done from pure css? For example, certainly
> highlighting one line in pure css based on the #anchor is possible,
> via css link selectors.

Maybe you are thinking something I'm missing, but there are three 
unrelated <td>s there that happen to be rendered vertically aligned. 
They're not on a single <tr> despite it may look like that.

-Andy

> Jason
> 


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI
  2018-06-27 18:07     ` Jason
@ 2018-06-27 23:24       ` andy
  2018-06-27 23:30         ` Jason
  0 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-27 23:24 UTC (permalink / raw)




On 06/28/2018 02:07 AM, Jason A. Donenfeld wrote:
> Nack. If we're going to add something like this, the github GUI for it
> is clearly superior and the right way of going about things, with the
> elegant [...] menu.

"Nack"...

Can you help me understand what is "clearly superior" and "right way" 
about the [...] menu in github?

Since you seem to be asking me to implement it?

What put me off it was there's only one useful option on it, and it 
requires and extra click.

-Andy


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI
  2018-06-27 23:24       ` andy
@ 2018-06-27 23:30         ` Jason
  2018-06-27 23:38           ` andy
  0 siblings, 1 reply; 73+ messages in thread
From: Jason @ 2018-06-27 23:30 UTC (permalink / raw)


On Thu, Jun 28, 2018 at 1:24 AM Andy Green <andy at warmcat.com> wrote:
> Can you help me understand what is "clearly superior" and "right way"
> about the [...] menu in github?
>
> Since you seem to be asking me to implement it?
>
> What put me off it was there's only one useful option on it, and it
> requires and extra click.

I think we should have all three of github's options -- copying the
range, copying a permalink to it, and a simple link to git blame. If
you think that's too arduous and only really care about copying, then
instead of [...] in the menu you can just put ? (or whatever other
glyph) as a small icon inside the range, as opposed to a large
slow-fading side link. Though, I still think the menu is better than
?, because with ? it's still unclear what precisely is being copied.

Jason


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI
  2018-06-27 23:30         ` Jason
@ 2018-06-27 23:38           ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-27 23:38 UTC (permalink / raw)




On 06/28/2018 07:30 AM, Jason A. Donenfeld wrote:
> On Thu, Jun 28, 2018 at 1:24 AM Andy Green <andy at warmcat.com> wrote:
>> Can you help me understand what is "clearly superior" and "right way"
>> about the [...] menu in github?
>>
>> Since you seem to be asking me to implement it?
>>
>> What put me off it was there's only one useful option on it, and it
>> requires and extra click.
> 
> I think we should have all three of github's options -- copying the
> range, copying a permalink to it, and a simple link to git blame. If
> you think that's too arduous and only really care about copying, then

I do care about general alignment with github.  I want to replace github 
with this as far as it goes.

> instead of [...] in the menu you can just put ? (or whatever other
> glyph) as a small icon inside the range, as opposed to a large
> slow-fading side link. Though, I still think the menu is better than
> ?, because with ? it's still unclear what precisely is being copied.

That's true... I used it the other day and even I got the impression it 
would copy the highlit region.

OK I will look at the whole burger menu thing.

-Andy

> Jason
> 


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH] cgit.css: add copyright lines
  2018-06-27 21:45       ` andy
@ 2018-06-28 23:58         ` andy
  0 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-28 23:58 UTC (permalink / raw)


The existing css file does not have any copyright
lines on it.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cgit.css b/cgit.css
index 05c4530..30c480a 100644
--- a/cgit.css
+++ b/cgit.css
@@ -1,3 +1,11 @@
+/* cgit.css: css styles for cgit
+ *
+ * Copyright (C) 2006-2018 cgit Development Team <cgit at lists.zx2c4.com>
+ *
+ * Licensed under GNU General Public License v2
+ *   (see COPYING for full license text)
+ */
+
 div#cgit {
 	padding: 0em;
 	margin: 0em;



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 0/7] line range highlight
  2018-06-20  9:39 Highlighting lines or line ranges in tree view andy
                   ` (6 preceding siblings ...)
  2018-06-26 11:25 ` [PATCH v5 0/6] line range highlight andy
@ 2018-06-29  1:39 ` andy
  2018-06-29  1:40   ` [PATCH v6 1/7] config: add js andy
                     ` (6 more replies)
  7 siblings, 7 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:39 UTC (permalink / raw)


The following series adds the ability to direct the browser
to highlight specific lines and ranges of lines in
/tree/, /source/, and /blame/ views, using the existing
#URLs.

As part of the implementation it adds a new cgit.js file
that is included in cgit page <head> along with a new
config "js" to specify its url, defaulting to "/cgit.js".
It can be disabled by setting the "js" config to an empty
string.

Highlit lines or ranges are centred vertically rather than
the default of scrolled to the top of the browser window
area.

You can click on the line number links to highlight
individual lines, or line ranges when an individual line
is already highlit.  If a range is highlit, clicking on
a line number link clears the range and just highlights
that line again.

When you click on a line number link, a sticky "burger" menu
(***) appears on the left of the link.  If you click on
this, a popup menu appears with the options

Copy Lines
Copy Link
View / Remove Blame (which depends if you are in /blame/)

Copy Lines copies the text in the line or range that is highlit
into the clipboard.  Copy Link copies the full #URL from the
browser URL bar.  View / Remove Blame toggles between /tree/
and /blame/ views at the same highlit range.

CSS transitions are used throughout.

v6 switches to the burger menu and introduces the popup menu
and related option.  There's some cleanup of the JS and
introduce a copyright notice at the top.

You can find the patches as a usable tree here

https://warmcat.com/git/cgit/log/

Examples:

https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n152
https://libwebsockets.org/git/libwebsockets/tree/lib/core/private.h#n39-47

---

Andy Green (7):
      config: add js
      cgit.js: line range highlight: introduce javascript
      cgit.js: line range highlight: make responsive to url changes
      cgit.js: line range highlight: improve vertical scroll logic
      line-range-highlight: onclick handler and range selection
      line-range-highlight: burger menu and popup menu
      line-range-highlight: copy text


 Makefile     |    1 
 cgit.c       |    3 
 cgit.css     |   78 ++++++++++++
 cgit.h       |    1 
 cgit.js      |  367 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 cgitrc.5.txt |    5 +
 ui-blame.c   |    2 
 ui-shared.c  |    5 +
 ui-tree.c    |    2 
 9 files changed, 462 insertions(+), 2 deletions(-)
 create mode 100644 cgit.js

--
Signature


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 1/7] config: add js
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
@ 2018-06-29  1:40   ` andy
  2018-06-29  6:14     ` list
  2018-06-29  1:40   ` [PATCH v6 2/7] cgit.js: line range highlight: introduce javascript andy
                     ` (5 subsequent siblings)
  6 siblings, 1 reply; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Setting the js path to an empty string disables
emitting the reference to it in the head section.

Signed-off-by: Andy Green <andy at warmcat.com>
Reviewed-by: John Keeping <john at keeping.me.uk>
---
 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgit.js      |    0 
 cgitrc.5.txt |    5 +++++
 ui-shared.c  |    5 +++++
 6 files changed, 15 insertions(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 137150c..de7e13e 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 999db9e..582416e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..2737008 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,11 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".  Setting this to an empty string will
+	disable generation of the link to this file in the head section.
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".
diff --git a/ui-shared.c b/ui-shared.c
index 74ace10..52723b3 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -798,6 +798,11 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	if (*ctx.cfg.js) {
+		html("<script type='text/javascript' src='");
+		html_attr(ctx.cfg.js);
+		html("'/></script>\n");
+	}
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 2/7] cgit.js: line range highlight: introduce javascript
  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  1:40   ` andy
  2018-06-29  1:40   ` [PATCH v6 3/7] cgit.js: line range highlight: make responsive to url changes andy
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


This adds a small css class, and a clientside js function plus
event registrations in cgit.js, to interpret the # part of the
URL on the client, and apply a highlight to filtered source.

Unlike blame highlight boxes which use generated divs, this
applies a computed absolutely-positioned, transparent div highlight
over the affected line(s) on the client side.

The # part of the URL is defined to not be passed to the server,
so the highlight can't be rendered on the server side.
However this has the advantage that the line range highlight
can operate on /blame/ urls trivially, since it doesn't
conflict with blame's generated div scheme.

pointer-events: none is used on the highlight overlay div to
allow the user to cut-and-paste in the highlit region and
click on links underneath normally.

The JS supports highlighting single lines as before like #n123
and also ranges of lines like #n123-135.

Because the browser can no longer automatically scroll to the
element in the second case, the JS also takes care of extracting
the range start element and scrolling to it dynamically.

Tested on Linux Firefox 60 + Linux Chrome 67

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |    9 ++++++
 cgit.js  |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/cgit.css b/cgit.css
index 1f581ec..45ba615 100644
--- a/cgit.css
+++ b/cgit.css
@@ -336,6 +336,7 @@ div#cgit table.ssdiff td.lineno a {
 	color: gray;
 	text-align: right;
 	text-decoration: none;
+	transition: background-color 1.5s;
 }
 
 div#cgit table.blob td.linenumbers a:hover,
@@ -376,6 +377,14 @@ div#cgit table.blame td.lines > div > pre {
 	top: 0;
 }
 
+div#cgit div.selected-lines {
+	position: absolute;
+	pointer-events: none;
+	z-index: 1;
+	background-color: rgba(255, 255, 0, 0);
+	transition: background-color 1.5s;
+}
+
 div#cgit table.bin-blob {
 	margin-top: 0.5em;
 	border: solid 1px black;
diff --git a/cgit.js b/cgit.js
index e69de29..7c034e3 100644
--- a/cgit.js
+++ b/cgit.js
@@ -0,0 +1,92 @@
+/* cgit.css: javacript functions for cgit
+ *
+ * Copyright (C) 2006-2018 cgit Development Team <cgit at lists.zx2c4.com>
+ *
+ * Licensed under GNU General Public License v2
+ *   (see COPYING for full license text)
+ */
+
+(function () {
+
+function collect_offsetTop(e1)
+{
+	var t = 0;
+
+	while (e1) {
+		if (e1.offsetTop)
+			t += e1.offsetTop;
+		e1 = e1.offsetParent;
+	}
+
+	return t;
+}
+
+function find_parent_of_type(e, type)
+{
+	while (e.tagName.toLowerCase() != type)
+		e = e.parentNode;
+
+	return e;
+}
+
+function line_range_highlight()
+{
+	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
+
+	l1 = parseInt(h.substring(2));
+	if (!l1)
+		return;
+
+	t = h.indexOf("-");
+	l2 = l1;
+	if (t >= 1)
+		l2 = parseInt(h.substring(t + 1));
+
+	if (l2 < l1)
+		l2 = l1;
+
+	var lh, etable, etr, de, n;
+
+	e = document.getElementById('n' + l1);
+	if (!e)
+		return;
+
+	de = document.createElement("DIV");
+
+	de.className = "selected-lines";
+	de.style.bottom = e.style.bottom;
+	de.style.top = collect_offsetTop(e) + 'px';
+	de.l1 = l1;
+	de.l2 = l2;
+
+	/* we will tack the highlight div at the parent tr */
+	etr = find_parent_of_type(e, "tr");
+
+	de.style.width = etr.offsetWidth + 'px';
+
+	/* the table is offset from the left, the highlight
+	 * needs to follow it */
+	etable = find_parent_of_type(etr, "table");
+
+	de.style.left = etable.offsetLeft + 'px';
+	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
+
+	etr.insertBefore(de, etr.firstChild);
+
+	setTimeout(function() {
+		de.style.backgroundColor = "rgba(255, 255, 0, 0.2)";
+	}, 1);
+
+	n = l1;
+	while (n <= l2)
+		document.getElementById('n' + n++).style.backgroundColor = "yellow";
+
+	e.scrollIntoView(true);
+}
+
+/* we have to use load, because header images can push the layout vertically */
+window.addEventListener("load", function() {
+	line_range_highlight();
+}, false);
+
+})();



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 3/7] cgit.js: line range highlight: make responsive to url changes
  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  1:40   ` [PATCH v6 2/7] cgit.js: line range highlight: introduce javascript andy
@ 2018-06-29  1:40   ` andy
  2018-06-29  1:40   ` [PATCH v6 4/7] cgit.js: line range highlight: improve vertical scroll logic andy
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


Browser default interpretation of the # part of the URL is
to try to match it to an element ID one time at page load.

Subsequent modifications in the URL bar are ignored, even if
you hit enter there.

This patch makes the line range highlight able to clean up
after itself and be reapplied, and has the highlight function
listen for hashchange events and re-interpret the # part
when they occur.

Now if you edit the URL and press enter, any existing highlight
is removed, the # part reinterpreted and the new highlight
applied automatically.

This is particularly useful if you edit the # link with the
intention to show a range before copying the URL.

Clicking on the line number, which changes the URL bar to
have a corresponding #n part, also triggers hashchange and
makes the clicked line be highlit immediately.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/cgit.js b/cgit.js
index 7c034e3..6cc27c1 100644
--- a/cgit.js
+++ b/cgit.js
@@ -33,6 +33,18 @@ function line_range_highlight()
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
+	e = document.getElementById("cgit-line-range");
+	if (e) {
+		l1 = e.l1;
+		while (l1 <= e.l2) {
+			var e1;
+			e1 = document.getElementById('n' + l1++);
+			e1.style.backgroundColor = null;
+		}
+
+		e.remove();
+	}
+
 	l1 = parseInt(h.substring(2));
 	if (!l1)
 		return;
@@ -56,6 +68,7 @@ function line_range_highlight()
 	de.className = "selected-lines";
 	de.style.bottom = e.style.bottom;
 	de.style.top = collect_offsetTop(e) + 'px';
+	de.id = "cgit-line-range";
 	de.l1 = l1;
 	de.l2 = l2;
 
@@ -89,4 +102,8 @@ window.addEventListener("load", function() {
 	line_range_highlight();
 }, false);
 
+window.addEventListener("hashchange", function() {
+	line_range_highlight();
+}, false);
+
 })();



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 4/7] cgit.js: line range highlight: improve vertical scroll logic
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
                     ` (2 preceding siblings ...)
  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   ` andy
  2018-06-29  1:40   ` [PATCH v6 5/7] line-range-highlight: onclick handler and range selection andy
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


Instead of following the browser heuristic to put any matching
id element to the top left of the browser window, compute the
number of visible lines vertically in the window, and the
middle of the highlit range, and try to centre the middle of
the highlit range in the window.

If the top of the range is no longer visible due to a range
consisting of more lines than the window can show, fall back to
placing the top of the range at the top of the window.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index 6cc27c1..d99c980 100644
--- a/cgit.js
+++ b/cgit.js
@@ -57,7 +57,7 @@ function line_range_highlight()
 	if (l2 < l1)
 		l2 = l1;
 
-	var lh, etable, etr, de, n;
+	var lh, etable, etr, de, n, hl, v;
 
 	e = document.getElementById('n' + l1);
 	if (!e)
@@ -94,7 +94,18 @@ function line_range_highlight()
 	while (n <= l2)
 		document.getElementById('n' + n++).style.backgroundColor = "yellow";
 
-	e.scrollIntoView(true);
+	hl = (window.innerHeight / (e.offsetHeight + 1));
+	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
+	if (v > l1)
+		v = l1;
+	if (v < 1)
+		v = 1;
+
+	t = document.getElementById('n' + Math.round(v));
+	if (!t)
+		t = e;
+
+	t.scrollIntoView(true);
 }
 
 /* we have to use load, because header images can push the layout vertically */



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 5/7] line-range-highlight: onclick handler and range selection
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
                     ` (3 preceding siblings ...)
  2018-06-29  1:40   ` [PATCH v6 4/7] cgit.js: line range highlight: improve vertical scroll logic andy
@ 2018-06-29  1:40   ` 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
  6 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


This allows the user to select line ranges simply by clicking on the
line number links.

 - No selected highlit line, or a range already selected, causes the
click to highlight just the clicked line as usual.

 - Clicking on a second line number link when a single line was already
highlit creates a line range highlight, between the lowest and
highest line numbers of the already-selected and newly-selected
line number links.

The order of the clicks is unimportant, you can click the higher
line number link first and then the lower to define the range of
lines equally well.

The implementation is slightly complicated by our single parent
onclick handler not being able to interrupt the already ongoing
processing of the a href #n change from the link click itself.

Rather than bloat every linenumber link with its own onclick handler
defeating this default we simply do it with a single parent
onclick event and apply any computed range url in the existing
hashchange event handler.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js    |   34 ++++++++++++++++++++++++++++++++++
 ui-blame.c |    2 +-
 ui-tree.c  |    2 +-
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/cgit.js b/cgit.js
index d99c980..015dd76 100644
--- a/cgit.js
+++ b/cgit.js
@@ -108,11 +108,45 @@ function line_range_highlight()
 	t.scrollIntoView(true);
 }
 
+function line_range_click(e) {
+	var t, m, n = window.location.href.length - window.location.hash.length;
+
+	/* disable passthru to stop needless scrolling by default browser #URL handler */
+	e.stopPropagation();
+	e.preventDefault();
+
+	if (!window.location.hash ||
+	    window.location.hash.indexOf("-") >= 0)
+		t = window.location.href.substring(0, n) +
+		    '#n' + e.target.id.substring(1);
+	else {
+		if (parseInt(window.location.hash.substring(2)) <
+		    parseInt(e.target.id.substring(1)))  /* forwards */
+			t = window.location + '-' + e.target.id.substring(1);
+		else
+			t = window.location.href.substring(0, n) +
+				'#n' + e.target.id.substring(1) + '-' +
+				window.location.href.substring(n + 2);
+	}
+
+	window.history.replaceState(null, null, t);
+
+	line_range_highlight();
+}
+
 /* we have to use load, because header images can push the layout vertically */
 window.addEventListener("load", function() {
 	line_range_highlight();
 }, false);
 
+document.addEventListener("DOMContentLoaded", function() {
+	/* event listener cannot override default #URL browser processing,
+	 * requires onclick */
+	var e = document.getElementById("linenumbers");
+	if (e)
+		e.onclick = line_range_click;
+}, false);
+
 window.addEventListener("hashchange", function() {
 	line_range_highlight();
 }, false);
diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..c9cca18 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -170,7 +170,7 @@ static void print_object(const struct object_id *oid, const char *path,
 
 	/* Line numbers */
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<td class='linenumbers'>");
+		html("<td id='linenumbers' class='linenumbers'>");
 		for (ent = sb.ent; ent; ent = ent->next) {
 			html("<div class='alt'><pre>");
 			emit_blame_entry_linenumber(ent);
diff --git a/ui-tree.c b/ui-tree.c
index e4cb558..f96b845 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -28,7 +28,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
 	html("<table summary='blob content' class='blob'>\n");
 
 	if (ctx.cfg.enable_tree_linenumbers) {
-		html("<tr><td class='linenumbers'><pre>");
+		html("<tr><td id='linenumbers' class='linenumbers'><pre>");
 		idx = 0;
 		lineno = 0;
 



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 6/7] line-range-highlight: burger menu and popup menu
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
                     ` (4 preceding siblings ...)
  2018-06-29  1:40   ` [PATCH v6 5/7] line-range-highlight: onclick handler and range selection andy
@ 2018-06-29  1:40   ` andy
  2018-06-29  1:40   ` [PATCH v6 7/7] line-range-highlight: copy text andy
  6 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


Clicking on the line numbers to control the highlight,
or visiting a #URL link, causes a burger menu to
appear on the left of either the top of the range
or the bottom if that was last clicked.

Clicking this brings up a popup menu with

Copy Lines (implemented in next patch)
Copy Link (copies complete #URL to clipboard)
View / Remove Blame (changes to view to have or not have blame)

Clicking outside the popup menu clears it, but the burger
menu is sticky.

This is an entirely clientside implementation in
cgit.css and cgit.js only.  If JS disabled at server
or at client, it cannot provide this functionality and
operates with one-line browser URLs as before.

Tested on Linux Chrome 67 + Firefox 60.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.css |   71 +++++++++++++++++++++
 cgit.js  |  206 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 264 insertions(+), 13 deletions(-)

diff --git a/cgit.css b/cgit.css
index 45ba615..093e0ce 100644
--- a/cgit.css
+++ b/cgit.css
@@ -382,7 +382,76 @@ div#cgit div.selected-lines {
 	pointer-events: none;
 	z-index: 1;
 	background-color: rgba(255, 255, 0, 0);
-	transition: background-color 1.5s;
+	transition: background-color 0.5s;
+}
+
+div#cgit div.selected-line-link-highlight {
+	background-color: yellow;
+}
+
+div#cgit div.selected-lines-popup {
+	position: absolute;
+	text-align: center;
+	font-size: 6px;
+	font-color: black;
+	left: 6px;
+	width: 24px;
+	height: 12px;
+	opacity: 0;
+	z-index: 2;
+	padding-top: 4px;
+	padding-bottom: 0px;
+	cursor: pointer;
+	filter: grayscale(100%);
+	border: 1px solid gray;
+	border-radius: 3px;
+	background-color: rgba(255, 255, 255, 0.0);
+	transition: opacity 0.5s;
+}
+
+div#cgit div.selected-lines-popup:before {
+	content: "\26AB\26AB\26AB";
+}
+
+div#cgit div.popup-menu {
+    display: inline-block;
+	position: relative;
+	text-align: left;
+	font-size: 12px;
+	color: black;
+	left: 8px;
+	opacity: 0;
+	z-index: 3;
+	margin: 3px;
+	padding: 3px;
+	cursor: pointer;
+	border: 1px solid gray;
+	border-radius: 3px;
+	background-color: white;
+	white-space: nowrap;
+	box-shadow: 0px 5px 15px gray;
+	transition: opacity 0.3s;
+}
+
+div#cgit div.popup-menu ul {
+	list-style-type: none;
+	left: 0;
+	margin: 3px;
+	padding: 3px;
+}
+
+div#cgit div.popup-menu li {
+	margin: 3px;
+	padding: 3px;
+	font-size: 14px;
+}
+
+div#cgit div.popup-menu li:active {
+	margin: 3px;
+	padding: 3px;
+	font-size: 14px;
+	background-color: blue;
+	color: white;
 }
 
 div#cgit table.bin-blob {
diff --git a/cgit.js b/cgit.js
index 015dd76..7fae9aa 100644
--- a/cgit.js
+++ b/cgit.js
@@ -8,6 +8,8 @@
 
 (function () {
 
+var burger, menu_popup;
+
 function collect_offsetTop(e1)
 {
 	var t = 0;
@@ -29,7 +31,15 @@ function find_parent_of_type(e, type)
 	return e;
 }
 
-function line_range_highlight()
+/*
+ * This creates an absolute div as a child of the content table.
+ * It's horizontally and vertically aligned and sized according
+ * to the #URL information like #n123-456
+ * 
+ * If the highlight div already exists, it's removed and remade.
+ */
+
+function line_range_highlight(do_burger)
 {
 	var h = window.location.hash, l1 = 0, l2 = 0, e, t;
 
@@ -39,7 +49,8 @@ function line_range_highlight()
 		while (l1 <= e.l2) {
 			var e1;
 			e1 = document.getElementById('n' + l1++);
-			e1.style.backgroundColor = null;
+				e1.classList.remove(
+					'selected-line-link-highlight');
 		}
 
 		e.remove();
@@ -63,6 +74,9 @@ function line_range_highlight()
 	if (!e)
 		return;
 
+	if (do_burger)
+		burger_create(e);
+
 	de = document.createElement("DIV");
 
 	de.className = "selected-lines";
@@ -77,10 +91,8 @@ function line_range_highlight()
 
 	de.style.width = etr.offsetWidth + 'px';
 
-	/* the table is offset from the left, the highlight
-	 * needs to follow it */
+	/* the table is offset from the left, the highlight needs to follow it */
 	etable = find_parent_of_type(etr, "table");
-
 	de.style.left = etable.offsetLeft + 'px';
 	de.style.height = ((l2 - l1 + 1) * e.offsetHeight) + 'px';
 
@@ -92,7 +104,8 @@ function line_range_highlight()
 
 	n = l1;
 	while (n <= l2)
-		document.getElementById('n' + n++).style.backgroundColor = "yellow";
+		document.getElementById('n' + n++).classList.add(
+					'selected-line-link-highlight');
 
 	hl = (window.innerHeight / (e.offsetHeight + 1));
 	v = (l1 + ((l2 - l1) / 2)) - (hl / 2);
@@ -108,15 +121,184 @@ function line_range_highlight()
 	t.scrollIntoView(true);
 }
 
+function copy_clipboard(value)
+{
+	var inp = document.createElement("textarea");
+	var e = document.getElementById("linenumbers");
+
+	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();
+}
+
+/*
+ * An element in the popup menu was clicked, perform the appropriate action
+ */
+function mi_click(e) {
+	var u, n;
+
+	e.stopPropagation();
+	e.preventDefault();
+
+	switch (e.target.id) {
+	case "mi-c-line":
+		/* implemented in next patch */
+		break;
+	case "mi-c-link":
+		copy_clipboard(window.location.href);
+		break;
+	case "mi-c-blame":
+		u = window.location.href;
+		t = u.indexOf("/tree/");
+		if (t)
+			window.location = u.substring(0, t) + "/blame/" +
+				u.substring(t + 6);
+		break;
+	case "mi-c-tree":
+		u = window.location.href;
+		t = u.indexOf("/blame/");
+		if (t)
+			window.location = u.substring(0, t) + "/tree/" +
+				u.substring(t + 7);
+		break;
+	}
+
+	if (!menu_popup)
+		return;
+
+	menu_popup.remove();
+	menu_popup = null;
+}
+
+/* We got a click on the (***) burger menu */
+
+function burger_click(e) {
+	var e1 = e, etable, d = new Date, s = "", n, is_blame,
+	    ar = new Array("mi-c-line", "mi-c-link", "mi-c-blame", "mi-c-tree"),
+	    an = new Array("Copy Lines", "Copy Link",
+			   "View Blame", /* 2: shown in /tree/ */
+			   "Remove Blame" /* 3: shown in /blame/ */);
+
+	e.preventDefault();
+
+	if (menu_popup) {
+		menu_popup.remove();
+		menu_popup = null;
+
+		return;
+	}
+
+	/*
+	 * Create the popup menu
+	 */
+
+	is_blame = !!document.getElementsByClassName("hashes").length;
+
+	menu_popup = document.createElement("DIV");
+	menu_popup.className = "popup-menu";
+	menu_popup.style.top = collect_offsetTop(e1) + e.offsetHeight + "px";
+
+	s = "<ul id='menu-ul'>";
+	for (n = 0; n < an.length; n++)
+		if (n < 2 || is_blame == (n == 3))
+			s += "<li id='" + ar[n] + "' tabindex='" + n + "'>" +
+				an[n] + "</li>";
+		    
+	menu_popup.innerHTML = s;
+
+	burger.insertBefore(menu_popup, null);
+
+        document.getElementById(ar[0]).focus();
+	for (n = 0; n < an.length; n++)
+		if (n < 2 || is_blame == (n == 3))
+			document.getElementById(ar[n]).
+				addEventListener("click", mi_click);
+				
+	setTimeout(function() {
+		menu_popup.style.opacity = "1";
+	}, 1);
+
+	/* detect loss of focus for popup menu */
+	menu_popup.addEventListener("focusout", function(e) {
+		/* if focus went to a child (menu item), ignore */
+		if (e.relatedTarget &&
+		    e.relatedTarget.parentNode.id == "menu-ul")
+			return;
+
+		menu_popup.remove();
+		menu_popup = null;
+	});
+}
+
+function burger_create(e)
+{
+	var e1 = e, etable, d = new Date;
+
+	if (burger)
+		burger.remove();
+
+	burger = document.createElement("DIV");
+	burger.className = "selected-lines-popup";
+	burger.style.top = collect_offsetTop(e1) + "px";
+
+	/* event listener cannot override default browser #URL behaviour */
+	burger.onclick = burger_click;
+
+	etable = find_parent_of_type(e, "table");
+	etable.insertBefore(burger, etable.firstChild);
+	burger_time = d.getTime();
+
+	setTimeout(function() {
+		burger.style.opacity = "1";
+	}, 1);
+}
+
+/*
+ * We got a click on a line number #url
+ *
+ * Create the "burger" menu there.
+ *
+ * Redraw the line range highlight accordingly.
+ */
+
 function line_range_click(e) {
-	var t, m, n = window.location.href.length - window.location.hash.length;
+	var t, elem, m, n = window.location.href.length -
+			    window.location.hash.length;
 
-	/* disable passthru to stop needless scrolling by default browser #URL handler */
+	/* disable passthru to stop scrolling by browser #URL handler */
 	e.stopPropagation();
 	e.preventDefault();
 
+	if (!e.target.id)
+		return;
+
+	if (menu_popup) {
+		menu_popup.remove();
+		menu_popup = null;
+
+		return;
+	}
+
+	elem = document.getElementById(e.target.id);
+	if (!elem)
+		return;
+
+	burger_create(elem);
+
 	if (!window.location.hash ||
-	    window.location.hash.indexOf("-") >= 0)
+	    window.location.hash.indexOf("-") >= 0 ||
+	    e.target.id.substring(1) == window.location.href.substring(n + 2))
 		t = window.location.href.substring(0, n) +
 		    '#n' + e.target.id.substring(1);
 	else {
@@ -131,12 +313,12 @@ function line_range_click(e) {
 
 	window.history.replaceState(null, null, t);
 
-	line_range_highlight();
+	line_range_highlight(0);
 }
 
 /* we have to use load, because header images can push the layout vertically */
 window.addEventListener("load", function() {
-	line_range_highlight();
+	line_range_highlight(1);
 }, false);
 
 document.addEventListener("DOMContentLoaded", function() {
@@ -148,7 +330,7 @@ document.addEventListener("DOMContentLoaded", function() {
 }, false);
 
 window.addEventListener("hashchange", function() {
-	line_range_highlight();
+	line_range_highlight(1);
 }, false);
 
 })();



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 7/7] line-range-highlight: copy text
  2018-06-29  1:39 ` [PATCH v6 0/7] line range highlight andy
                     ` (5 preceding siblings ...)
  2018-06-29  1:40   ` [PATCH v6 6/7] line-range-highlight: burger menu and popup menu andy
@ 2018-06-29  1:40   ` andy
  6 siblings, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  1:40 UTC (permalink / raw)


Implement picking a line range or single line
from the textContent version of the rendered
source to the clipboard.

It works in blame and tree the same.

Signed-off-by: Andy Green <andy at warmcat.com>
---
 cgit.js |   51 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/cgit.js b/cgit.js
index 7fae9aa..bc2ba65 100644
--- a/cgit.js
+++ b/cgit.js
@@ -31,6 +31,25 @@ function find_parent_of_type(e, type)
 	return e;
 }
 
+function parse_hashurl_start(h)
+{
+	return parseInt(h.substring(2));
+}
+
+function parse_hashurl_end(h, start)
+{
+	var t = h.indexOf("-"), e = start;
+
+	if (t >= 1)
+		e = parseInt(h.substring(t + 1));
+
+	if (e < start)
+		e = start;
+
+	return e;
+}
+
+
 /*
  * This creates an absolute div as a child of the content table.
  * It's horizontally and vertically aligned and sized according
@@ -56,17 +75,11 @@ function line_range_highlight(do_burger)
 		e.remove();
 	}
 
-	l1 = parseInt(h.substring(2));
+	l1 = parse_hashurl_start(h);
 	if (!l1)
 		return;
 
-	t = h.indexOf("-");
-	l2 = l1;
-	if (t >= 1)
-		l2 = parseInt(h.substring(t + 1));
-
-	if (l2 < l1)
-		l2 = l1;
+	l2 = parse_hashurl_end(h, l1);
 
 	var lh, etable, etr, de, n, hl, v;
 
@@ -142,18 +155,36 @@ function copy_clipboard(value)
 	inp.remove();
 }
 
+/* we want to copy plain text for a line range */
+
+function copy_text(elem, l1, l2)
+{
+	var tc = elem.textContent.split('\n'), s = "";
+
+	l1 --;
+
+	while (l1 < l2)
+		s += tc[l1++] + '\n';
+
+	copy_clipboard(s);
+}
+
+
 /*
  * An element in the popup menu was clicked, perform the appropriate action
  */
 function mi_click(e) {
-	var u, n;
+	var u, n, l1, l2, el;
 
 	e.stopPropagation();
 	e.preventDefault();
 
 	switch (e.target.id) {
 	case "mi-c-line":
-		/* implemented in next patch */
+		l1 = parse_hashurl_start(window.location.hash);
+		l2 = parse_hashurl_end(window.location.hash, l1);
+		el = document.getElementsByClassName("highlight")[0].firstChild;
+		copy_text(el, l1, l2);
 		break;
 	case "mi-c-link":
 		copy_clipboard(window.location.href);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6 1/7] config: add js
  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
  0 siblings, 1 reply; 73+ messages in thread
From: list @ 2018-06-29  6:14 UTC (permalink / raw)


Andy Green <andy at warmcat.com> on Fri, 2018/06/29 09:40:
> Just like the config allows setting css URL path,
> add a config for setting the js URL path
> 
> Setting the js path to an empty string disables
> emitting the reference to it in the head section.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
>  Makefile     |    1 +
>  cgit.c       |    3 +++
>  cgit.h       |    1 +
>  cgit.js      |    0 
>  cgitrc.5.txt |    5 +++++
>  ui-shared.c  |    5 +++++
>  6 files changed, 15 insertions(+)
>  create mode 100644 cgit.js
> 
> diff --git a/Makefile b/Makefile
> index 137150c..de7e13e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -87,6 +87,7 @@ install: all
>  	$(INSTALL) -m 0755 cgit
> $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME) $(INSTALL) -m 0755 -d
> $(DESTDIR)$(CGIT_DATA_PATH) $(INSTALL) -m 0644 cgit.css
> $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
> +	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
>  	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
>  	$(INSTALL) -m 0644 favicon.ico
> $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico $(INSTALL) -m 0644 robots.txt
> $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt diff --git a/cgit.c b/cgit.c
> index bdb2fad..8b23c8f 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -146,6 +146,8 @@ static void config_cb(const char *name, const char
> *value) ctx.cfg.root_readme = xstrdup(value);
>  	else if (!strcmp(name, "css"))
>  		ctx.cfg.css = xstrdup(value);
> +	else if (!strcmp(name, "js"))
> +		ctx.cfg.js = xstrdup(value);
>  	else if (!strcmp(name, "favicon"))
>  		ctx.cfg.favicon = xstrdup(value);
>  	else if (!strcmp(name, "footer"))
> @@ -384,6 +386,7 @@ static void prepare_context(void)
>  	ctx.cfg.branch_sort = 0;
>  	ctx.cfg.commit_sort = 0;
>  	ctx.cfg.css = "/cgit.css";
> +	ctx.cfg.js = "/cgit.js";
>  	ctx.cfg.logo = "/cgit.png";
>  	ctx.cfg.favicon = "/favicon.ico";
>  	ctx.cfg.local_time = 0;
> diff --git a/cgit.h b/cgit.h
> index 999db9e..582416e 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -194,6 +194,7 @@ struct cgit_config {
>  	char *clone_prefix;
>  	char *clone_url;
>  	char *css;
> +	char *js;
>  	char *favicon;
>  	char *footer;
>  	char *head_include;
> diff --git a/cgit.js b/cgit.js
> new file mode 100644
> index 0000000..e69de29
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 99fc799..2737008 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -248,6 +248,11 @@ inline-readme::
>  	individually also choose to ignore this global list, and create a
>  	repo-specific list by using 'repo.inline-readme'.
>  
> +js::
> +	Url which specifies the javascript script document to include in
> all cgit
> +	pages.  Default value: "/cgit.js".  Setting this to an empty
> string will
> +	disable generation of the link to this file in the head section.
> +
>  local-time::
>  	Flag which, if set to "1", makes cgit print commit and tag times
> in the servers timezone. Default value: "0".
> diff --git a/ui-shared.c b/ui-shared.c
> index 74ace10..52723b3 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -798,6 +798,11 @@ void cgit_print_docstart(void)
>  	html("<link rel='stylesheet' type='text/css' href='");
>  	html_attr(ctx.cfg.css);
>  	html("'/>\n");  
> +	if (*ctx.cfg.js) {
> +		html("<script type='text/javascript' src='");
> +		html_attr(ctx.cfg.js);
> +		html("'/></script>\n");

The opening tag still has a slash that should not be there.

> +	}
>  	if (ctx.cfg.favicon) {
>  		html("<link rel='shortcut icon' href='");
>  		html_attr(ctx.cfg.favicon);
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit



-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20180629/a97ba1b7/attachment.asc>


^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6-ninjaedit] config: add js
  2018-06-29  6:14     ` list
@ 2018-06-29  6:16       ` andy
  2018-06-29  6:31         ` [PATCH v6-ninjaedit2] " andy
  2018-06-29  6:33         ` [PATCH v6-ninjaedit] " list
  0 siblings, 2 replies; 73+ messages in thread
From: andy @ 2018-06-29  6:16 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Setting the js path to an empty string disables
emitting the reference to it in the head section.

Signed-off-by: Andy Green <andy at warmcat.com>
Reviewed-by: John Keeping <john at keeping.me.uk>
---
 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgit.js      |    0 
 cgitrc.5.txt |    5 +++++
 ui-shared.c  |    5 +++++
 6 files changed, 15 insertions(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 137150c..de7e13e 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 999db9e..582416e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..2737008 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,11 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".  Setting this to an empty string will
+	disable generation of the link to this file in the head section.
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".
diff --git a/ui-shared.c b/ui-shared.c
index 74ace10..ff16cbd 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -798,6 +798,11 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	if (*ctx.cfg.js) {
+		html("<script type='text/javascript' src='");
+		html_attr(ctx.cfg.js);
+		html("'/>\n");
+	}
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6-ninjaedit2] config: add js
  2018-06-29  6:16       ` [PATCH v6-ninjaedit] " andy
@ 2018-06-29  6:31         ` andy
  2018-06-29  6:33         ` [PATCH v6-ninjaedit] " list
  1 sibling, 0 replies; 73+ messages in thread
From: andy @ 2018-06-29  6:31 UTC (permalink / raw)


Just like the config allows setting css URL path,
add a config for setting the js URL path

Setting the js path to an empty string disables
emitting the reference to it in the head section.

Signed-off-by: Andy Green <andy at warmcat.com>
Reviewed-by: John Keeping <john at keeping.me.uk>
---
 Makefile     |    1 +
 cgit.c       |    3 +++
 cgit.h       |    1 +
 cgit.js      |    0 
 cgitrc.5.txt |    5 +++++
 ui-shared.c  |    5 +++++
 6 files changed, 15 insertions(+)
 create mode 100644 cgit.js

diff --git a/Makefile b/Makefile
index 137150c..de7e13e 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ install: all
 	$(INSTALL) -m 0755 cgit $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME)
 	$(INSTALL) -m 0755 -d $(DESTDIR)$(CGIT_DATA_PATH)
 	$(INSTALL) -m 0644 cgit.css $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
+	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
 	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
 	$(INSTALL) -m 0644 favicon.ico $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico
 	$(INSTALL) -m 0644 robots.txt $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt
diff --git a/cgit.c b/cgit.c
index bdb2fad..8b23c8f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -146,6 +146,8 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.root_readme = xstrdup(value);
 	else if (!strcmp(name, "css"))
 		ctx.cfg.css = xstrdup(value);
+	else if (!strcmp(name, "js"))
+		ctx.cfg.js = xstrdup(value);
 	else if (!strcmp(name, "favicon"))
 		ctx.cfg.favicon = xstrdup(value);
 	else if (!strcmp(name, "footer"))
@@ -384,6 +386,7 @@ static void prepare_context(void)
 	ctx.cfg.branch_sort = 0;
 	ctx.cfg.commit_sort = 0;
 	ctx.cfg.css = "/cgit.css";
+	ctx.cfg.js = "/cgit.js";
 	ctx.cfg.logo = "/cgit.png";
 	ctx.cfg.favicon = "/favicon.ico";
 	ctx.cfg.local_time = 0;
diff --git a/cgit.h b/cgit.h
index 999db9e..582416e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -194,6 +194,7 @@ struct cgit_config {
 	char *clone_prefix;
 	char *clone_url;
 	char *css;
+	char *js;
 	char *favicon;
 	char *footer;
 	char *head_include;
diff --git a/cgit.js b/cgit.js
new file mode 100644
index 0000000..e69de29
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 99fc799..2737008 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -248,6 +248,11 @@ inline-readme::
 	individually also choose to ignore this global list, and create a
 	repo-specific list by using 'repo.inline-readme'.
 
+js::
+	Url which specifies the javascript script document to include in all cgit
+	pages.  Default value: "/cgit.js".  Setting this to an empty string will
+	disable generation of the link to this file in the head section.
+
 local-time::
 	Flag which, if set to "1", makes cgit print commit and tag times in the
 	servers timezone. Default value: "0".
diff --git a/ui-shared.c b/ui-shared.c
index 74ace10..e1ff349 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -798,6 +798,11 @@ void cgit_print_docstart(void)
 	html("<link rel='stylesheet' type='text/css' href='");
 	html_attr(ctx.cfg.css);
 	html("'/>\n");
+	if (*ctx.cfg.js) {
+		html("<script type='text/javascript' src='");
+		html_attr(ctx.cfg.js);
+		html("'></script>\n");
+	}
 	if (ctx.cfg.favicon) {
 		html("<link rel='shortcut icon' href='");
 		html_attr(ctx.cfg.favicon);



^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH v6-ninjaedit] config: add js
  2018-06-29  6:16       ` [PATCH v6-ninjaedit] " andy
  2018-06-29  6:31         ` [PATCH v6-ninjaedit2] " andy
@ 2018-06-29  6:33         ` list
  1 sibling, 0 replies; 73+ messages in thread
From: list @ 2018-06-29  6:33 UTC (permalink / raw)


Andy Green <andy at warmcat.com> on Fri, 2018/06/29 14:16:
> Just like the config allows setting css URL path,
> add a config for setting the js URL path
> 
> Setting the js path to an empty string disables
> emitting the reference to it in the head section.
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
>  Makefile     |    1 +
>  cgit.c       |    3 +++
>  cgit.h       |    1 +
>  cgit.js      |    0 
>  cgitrc.5.txt |    5 +++++
>  ui-shared.c  |    5 +++++
>  6 files changed, 15 insertions(+)
>  create mode 100644 cgit.js
> 
> diff --git a/Makefile b/Makefile
> index 137150c..de7e13e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -87,6 +87,7 @@ install: all
>  	$(INSTALL) -m 0755 cgit
> $(DESTDIR)$(CGIT_SCRIPT_PATH)/$(CGIT_SCRIPT_NAME) $(INSTALL) -m 0755 -d
> $(DESTDIR)$(CGIT_DATA_PATH) $(INSTALL) -m 0644 cgit.css
> $(DESTDIR)$(CGIT_DATA_PATH)/cgit.css
> +	$(INSTALL) -m 0644 cgit.js $(DESTDIR)$(CGIT_DATA_PATH)/cgit.js
>  	$(INSTALL) -m 0644 cgit.png $(DESTDIR)$(CGIT_DATA_PATH)/cgit.png
>  	$(INSTALL) -m 0644 favicon.ico
> $(DESTDIR)$(CGIT_DATA_PATH)/favicon.ico $(INSTALL) -m 0644 robots.txt
> $(DESTDIR)$(CGIT_DATA_PATH)/robots.txt diff --git a/cgit.c b/cgit.c
> index bdb2fad..8b23c8f 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -146,6 +146,8 @@ static void config_cb(const char *name, const char
> *value) ctx.cfg.root_readme = xstrdup(value);
>  	else if (!strcmp(name, "css"))
>  		ctx.cfg.css = xstrdup(value);
> +	else if (!strcmp(name, "js"))
> +		ctx.cfg.js = xstrdup(value);
>  	else if (!strcmp(name, "favicon"))
>  		ctx.cfg.favicon = xstrdup(value);
>  	else if (!strcmp(name, "footer"))
> @@ -384,6 +386,7 @@ static void prepare_context(void)
>  	ctx.cfg.branch_sort = 0;
>  	ctx.cfg.commit_sort = 0;
>  	ctx.cfg.css = "/cgit.css";
> +	ctx.cfg.js = "/cgit.js";
>  	ctx.cfg.logo = "/cgit.png";
>  	ctx.cfg.favicon = "/favicon.ico";
>  	ctx.cfg.local_time = 0;
> diff --git a/cgit.h b/cgit.h
> index 999db9e..582416e 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -194,6 +194,7 @@ struct cgit_config {
>  	char *clone_prefix;
>  	char *clone_url;
>  	char *css;
> +	char *js;
>  	char *favicon;
>  	char *footer;
>  	char *head_include;
> diff --git a/cgit.js b/cgit.js
> new file mode 100644
> index 0000000..e69de29
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 99fc799..2737008 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -248,6 +248,11 @@ inline-readme::
>  	individually also choose to ignore this global list, and create a
>  	repo-specific list by using 'repo.inline-readme'.
>  
> +js::
> +	Url which specifies the javascript script document to include in
> all cgit
> +	pages.  Default value: "/cgit.js".  Setting this to an empty
> string will
> +	disable generation of the link to this file in the head section.
> +
>  local-time::
>  	Flag which, if set to "1", makes cgit print commit and tag times
> in the servers timezone. Default value: "0".
> diff --git a/ui-shared.c b/ui-shared.c
> index 74ace10..ff16cbd 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -798,6 +798,11 @@ void cgit_print_docstart(void)
>  	html("<link rel='stylesheet' type='text/css' href='");
>  	html_attr(ctx.cfg.css);
>  	html("'/>\n");  
> +	if (*ctx.cfg.js) {
> +		html("<script type='text/javascript' src='");
> +		html_attr(ctx.cfg.js);
> +		html("'/>\n");

That results is a white page. I think the line should read:

+		html("'></script>\n");

> +	}
>  	if (ctx.cfg.favicon) {
>  		html("<link rel='shortcut icon' href='");
>  		html_attr(ctx.cfg.favicon);
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit



-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20180629/ff171a72/attachment.asc>


^ permalink raw reply	[flat|nested] 73+ messages in thread

end of thread, other threads:[~2018-06-29  6:33 UTC | newest]

Thread overview: 73+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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).