Skip to content

Commit e6e0d8f

Browse files
larseggertlarseggert
authored
feat: Diff arbitrary versions from new HTMLization page (ietf-tools#4863)
* feat: Diff arbitrary versions from new HTMLization page Fixes ietf-tools#4859 * Rework this based on @rjsparks' suggestion. Not quite done yet. * Progress * Fix HTML * Don't show compare buttons if there aren't at least two versions * Remove spurious title attribute * Use and style select2 for the version diff dropdowns * Roll in code review suggestions * Some tests!
1 parent 286e737 commit e6e0d8f

8 files changed

Lines changed: 204 additions & 117 deletions

File tree

‎ietf/doc/tests.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,12 @@ def test_document_draft(self):
744744
q = PyQuery(r.content)
745745
self.assertEqual(q('title').text(), 'draft-ietf-mars-test-01')
746746

747+
# check that revision list has expected versions
748+
self.assertEqual(len(q('#sidebar .revision-list .page-item.active a.page-link[href$="draft-ietf-mars-test-01"]')), 1)
749+
750+
# check that diff dropdowns have expected versions
751+
self.assertEqual(len(q('#sidebar option[value="draft-ietf-mars-test-00"][selected="selected"]')), 1)
752+
747753
rfc = WgRfcFactory()
748754
(Path(settings.RFC_PATH) / rfc.get_base_name()).touch()
749755
r = self.client.get(urlreverse("ietf.doc.views_doc.document_html", kwargs=dict(name=rfc.canonical_name())))

‎ietf/doc/views_doc.py‎

Lines changed: 68 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,7 @@ def document_main(request, name, rev=None, document_html=False):
533533
review_assignments=review_assignments,
534534
no_review_from_teams=no_review_from_teams,
535535
due_date=due_date,
536+
diff_revisions=get_diff_revisions(request, name, doc if isinstance(doc,Document) else doc.doc) if document_html else None
536537
))
537538

538539
if doc.type_id == "charter":
@@ -901,44 +902,77 @@ def document_email(request,name):
901902
)
902903

903904

904-
def document_history(request, name):
905-
doc = get_object_or_404(Document, docalias__name=name)
906-
top = render_document_top(request, doc, "history", name)
905+
def get_diff_revisions(request, name, doc):
906+
diffable = any(
907+
[
908+
name.startswith(prefix)
909+
for prefix in [
910+
"rfc",
911+
"draft",
912+
"charter",
913+
"conflict-review",
914+
"status-change",
915+
]
916+
]
917+
)
918+
919+
if not diffable:
920+
return []
907921

908922
# pick up revisions from events
909923
diff_revisions = []
910924

911-
diffable = [ name.startswith(prefix) for prefix in ["rfc", "draft", "charter", "conflict-review", "status-change", ]]
912-
if any(diffable):
913-
diff_documents = [ doc ]
914-
diff_documents.extend(Document.objects.filter(docalias__relateddocument__source=doc, docalias__relateddocument__relationship="replaces"))
915-
916-
if doc.get_state_slug() == "rfc":
917-
e = doc.latest_event(type="published_rfc")
918-
aliases = doc.docalias.filter(name__startswith="rfc")
919-
if aliases:
920-
name = aliases[0].name
921-
diff_revisions.append((name, "", e.time if e else doc.time, name))
922-
923-
seen = set()
924-
for e in NewRevisionDocEvent.objects.filter(type="new_revision", doc__in=diff_documents).select_related('doc').order_by("-time", "-id"):
925-
if (e.doc.name, e.rev) in seen:
926-
continue
927-
928-
seen.add((e.doc.name, e.rev))
929-
930-
url = ""
931-
if name.startswith("charter"):
932-
url = request.build_absolute_uri(urlreverse('ietf.doc.views_charter.charter_with_milestones_txt', kwargs=dict(name=e.doc.name, rev=e.rev)))
933-
elif name.startswith("conflict-review"):
934-
url = find_history_active_at(e.doc, e.time).get_href()
935-
elif name.startswith("status-change"):
936-
url = find_history_active_at(e.doc, e.time).get_href()
937-
elif name.startswith("draft") or name.startswith("rfc"):
938-
# rfcdiff tool has special support for IDs
939-
url = e.doc.name + "-" + e.rev
940-
941-
diff_revisions.append((e.doc.name, e.rev, e.time, url))
925+
diff_documents = [doc]
926+
diff_documents.extend(
927+
Document.objects.filter(
928+
docalias__relateddocument__source=doc,
929+
docalias__relateddocument__relationship="replaces",
930+
)
931+
)
932+
933+
if doc.get_state_slug() == "rfc":
934+
e = doc.latest_event(type="published_rfc")
935+
aliases = doc.docalias.filter(name__startswith="rfc")
936+
if aliases:
937+
name = aliases[0].name
938+
diff_revisions.append((name, "", e.time if e else doc.time, name))
939+
940+
seen = set()
941+
for e in (
942+
NewRevisionDocEvent.objects.filter(type="new_revision", doc__in=diff_documents)
943+
.select_related("doc")
944+
.order_by("-time", "-id")
945+
):
946+
if (e.doc.name, e.rev) in seen:
947+
continue
948+
949+
seen.add((e.doc.name, e.rev))
950+
951+
url = ""
952+
if name.startswith("charter"):
953+
url = request.build_absolute_uri(
954+
urlreverse(
955+
"ietf.doc.views_charter.charter_with_milestones_txt",
956+
kwargs=dict(name=e.doc.name, rev=e.rev),
957+
)
958+
)
959+
elif name.startswith("conflict-review"):
960+
url = find_history_active_at(e.doc, e.time).get_href()
961+
elif name.startswith("status-change"):
962+
url = find_history_active_at(e.doc, e.time).get_href()
963+
elif name.startswith("draft") or name.startswith("rfc"):
964+
# rfcdiff tool has special support for IDs
965+
url = e.doc.name + "-" + e.rev
966+
967+
diff_revisions.append((e.doc.name, e.rev, e.time, url))
968+
969+
return diff_revisions
970+
971+
972+
def document_history(request, name):
973+
doc = get_object_or_404(Document, docalias__name=name)
974+
top = render_document_top(request, doc, "history", name)
975+
diff_revisions = get_diff_revisions(request, name, doc)
942976

943977
# grab event history
944978
events = doc.docevent_set.all().order_by("-time", "-id").select_related("by")

‎ietf/static/css/document_html.scss‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,15 @@ tbody.meta tr {
206206
}
207207
}
208208

209+
.navbar {
210+
211+
td:not(:first-child),
212+
th:not(:first-child) {
213+
padding-top: map.get($spacers, 3);
214+
}
215+
216+
}
217+
209218
// Add some padding when there are multiple buttons in a line that can wrap
210219
.buttonlist .btn {
211220
margin-bottom: map.get($spacers, 1);
@@ -318,3 +327,15 @@ tbody.meta tr {
318327
display: none;
319328
}
320329
}
330+
331+
// Select2 styling
332+
@import "select2";
333+
334+
.select2-results__option,
335+
.select2-search__field {
336+
font-size: small !important;
337+
}
338+
339+
.select2-container--open {
340+
z-index: 9999999;
341+
}

‎ietf/static/js/document_html.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88

99
import Cookies from "js-cookie";
1010
import { populate_nav } from "./nav.js";
11+
import "./select2.js";
1112

1213
const cookies = Cookies.withAttributes({ sameSite: "strict" });
1314

@@ -48,7 +49,7 @@ document.addEventListener("DOMContentLoaded", function (event) {
4849

4950
// activate pref buttons selected by pref cookies or localStorage
5051
const in_localStorage = ["deftab"];
51-
document.querySelectorAll(".btn-check")
52+
document.querySelectorAll("#pref-tab-pane .btn-check")
5253
.forEach(btn => {
5354
const id = btn.id.replace("-radio", "");
5455

‎ietf/static/js/select2.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,11 @@ $(document)
109109
return;
110110
setupSelect2Field($(this));
111111
});
112+
113+
// Remove spurious title attribute (https://github.com/select2/select2/pull/3988)
114+
$(".select2-selection__rendered")
115+
.hover(function () {
116+
$(this)
117+
.removeAttr("title");
118+
});
112119
});

‎ietf/templates/doc/document_history.html‎

Lines changed: 1 addition & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -18,85 +18,7 @@
1818
{{ top|safe }}
1919
{% if diff_revisions and diff_revisions|length > 1 or doc.name|rfcbis %}
2020
<h2 class="my-3">Revision differences</h2>
21-
<form class="form-horizontal diff-form"
22-
action="{{ rfcdiff_base_url }}"
23-
method="get"
24-
target="_blank">
25-
<div class="row mb-3">
26-
<label for="url1" class="col-form-label col-sm-2 fw-bold">From revision</label>
27-
<div class="col-sm-10">
28-
<select class="form-select select2-field" data-max-entries="1" data-minimum-input-length="0" id="url1" name="url1">
29-
{% for name, rev, time, url in diff_revisions %}
30-
<option value="{{ url }}"
31-
{% if diff_revisions|length > 1 and forloop.counter == 2 %} selected="selected"{% endif %}>
32-
{{ name }}
33-
{% if rev %}-{{ rev }}{% endif %}
34-
({{ time|date:"Y-m-d" }})
35-
</option>
36-
{% endfor %}
37-
{% if doc.name|rfcbis %}
38-
<option value="{{ doc.name|rfcbis }}"
39-
{% if diff_revisions and diff_revisions|length == 1 %} selected="selected"{% endif %}>
40-
{{ doc.name|rfcbis }}
41-
</option>
42-
{% endif %}
43-
</select>
44-
</div>
45-
</div>
46-
<div class="row mb-3">
47-
<label for="url2" class="col-form-label col-sm-2 fw-bold">To revision</label>
48-
<div class="col-sm-10">
49-
<select class="form-select select2-field" data-max-entries="1" data-minimum-input-length="0" id="url2" name="url2">
50-
{% for name, rev, time, url in diff_revisions %}
51-
<option value="{{ url }}"
52-
{% if forloop.counter == 1 %} selected="selected"{% endif %}>
53-
{{ name }}
54-
{% if rev %}-{{ rev }}{% endif %}
55-
({{ time|date:"Y-m-d" }})
56-
</option>
57-
{% endfor %}
58-
{% if doc.name|rfcbis %}
59-
<option value="{{ doc.name|rfcbis }}">
60-
{{ doc.name|rfcbis }}
61-
</option>
62-
{% endif %}
63-
</select>
64-
</div>
65-
</div>
66-
<div class="row mb-3">
67-
<label class="col-form-label col-sm-2 fw-bold">Diff format</label>
68-
<div class="col-sm-10">
69-
<div class="btn-group" data-bs-toggle="buttons">
70-
<input type="radio"
71-
class="btn-check"
72-
checked
73-
name="difftype"
74-
value="--html"
75-
id="html">
76-
<label for="html" class="btn btn-outline-primary">Side-by-side</label>
77-
<input type="radio"
78-
class="btn-check"
79-
name="difftype"
80-
value="--abdiff"
81-
id="abdiff">
82-
<label for="abdiff" class="btn btn-outline-primary">Before-after</label>
83-
<input type="radio"
84-
class="btn-check"
85-
name="difftype"
86-
value="--chbars"
87-
id="chbars">
88-
<label for="chbars" class="btn btn-outline-primary">Change bars</label>
89-
<input type="radio"
90-
class="btn-check"
91-
name="difftype"
92-
value="--hwdiff"
93-
id="hwdiff">
94-
<label for="hwdiff" class="btn btn-outline-primary">Wdiff</label>
95-
</div>
96-
</div>
97-
</div>
98-
<button type="submit" class="btn btn-primary mb-3">Submit</button>
99-
</form>
21+
{% include "doc/document_history_form.html" with doc=doc diff_revisions=diff_revisions action=rfcdiff_base_url only %}
10022
{% endif %}
10123
<h2 class="my-3">Document history</h2>
10224
{% if can_add_comment %}

‎ietf/templates/doc/document_history_form.html‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
{# Copyright The IETF Trust 2015-2022, All Rights Reserved #}
2+
{% load origin %}
3+
{% load ietf_filters %}
4+
{% origin %}
5+
<form class="form-horizontal diff-form"
6+
action="{{ action }}"
7+
method="get"
8+
target="_blank">
9+
{% if not document_html %}
10+
<div class="row mb-3">
11+
<label for="url1" class="col-form-label col-sm-2 fw-bold">From revision</label>
12+
<div class="col-sm-10">
13+
{% endif %}
14+
<select class="form-select{% if document_html %} form-select-sm mb-1{% endif %} select2-field"
15+
data-max-entries="1"
16+
data-allow-clear="false"
17+
data-minimum-input-length="0"
18+
{% if not document_html %}id="url1"{% else %}aria-label="From revision"{% endif %}
19+
name="url1">
20+
{% for name, rev, time, url in diff_revisions %}
21+
<option value="{{ url }}"
22+
{% if diff_revisions|length > 1 and forloop.counter == 2 %} selected="selected"{% endif %}>
23+
{{ name|prettystdname }}{% if rev %}-{{ rev }}{% endif %}
24+
{% if not document_html %}({{ time|date:"Y-m-d" }}){% endif %}
25+
</option>
26+
{% endfor %}
27+
{% if doc.name|rfcbis %}
28+
<option value="{{ doc.name|rfcbis }}"
29+
{% if diff_revisions and diff_revisions|length == 1 %} selected="selected"{% endif %}>
30+
{{ doc.name|rfcbis|prettystdname }}
31+
</option>
32+
{% endif %}
33+
</select>
34+
{% if not document_html %}
35+
</div>
36+
</div>
37+
<div class="row mb-3">
38+
<label for="url2" class="col-form-label col-sm-2 fw-bold">To revision</label>
39+
<div class="col-sm-10">
40+
{% endif %}
41+
<select class="form-select{% if document_html %} form-select-sm mb-1{% endif %} select2-field"
42+
data-max-entries="1"
43+
data-allow-clear="false"
44+
data-minimum-input-length="0"
45+
{% if not document_html %}id="url2"{% else %}aria-label="To revision"{% endif %}
46+
name="url2">
47+
{% for name, rev, time, url in diff_revisions %}
48+
<option value="{{ url }}"
49+
{% if forloop.counter == 1 %} selected="selected"{% endif %}>
50+
{{ name|prettystdname }}{% if rev %}-{{ rev }}{% endif %}
51+
{% if not document_html %}({{ time|date:"Y-m-d" }}){% endif %}
52+
</option>
53+
{% endfor %}
54+
{% if doc.name|rfcbis %}
55+
<option value="{{ doc.name|rfcbis }}">
56+
{{ doc.name|rfcbis|prettystdname }}
57+
</option>
58+
{% endif %}
59+
</select>
60+
{% if not document_html %}
61+
</div>
62+
</div>
63+
<div class="row mb-3">
64+
<label class="col-form-label col-sm-2 fw-bold">Diff format</label>
65+
<div class="col-sm-10">
66+
{% endif %}
67+
<button type="submit"
68+
class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
69+
value="--html"
70+
name="difftype">
71+
Side-by-side
72+
</button>
73+
{% if not document_html %}
74+
<button type="submit"
75+
class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
76+
value="--abdiff"
77+
name="difftype">
78+
Before-after
79+
</button>
80+
<button type="submit"
81+
class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
82+
value="--chbars"
83+
name="difftype">
84+
Change bars
85+
</button>
86+
{% endif %}
87+
<button type="submit"
88+
class="btn btn-primary{% if document_html %} btn-sm{% endif %}"
89+
value="--hwdiff"
90+
name="difftype">
91+
Inline
92+
</button>
93+
{% if not document_html %}
94+
</div>
95+
</div>
96+
{% endif %}
97+
</form>

‎ietf/templates/doc/document_info.html‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,13 @@
7979
{% include "doc/revisions_list.html" with document_html=document_html %}
8080
</td>
8181
</tr>
82-
{% if doc.rev != "00" %}
82+
{% if diff_revisions|length > 1 %}
8383
<tr>
8484
<td></td>
8585
<th scope="row">Compare versions</th>
8686
<td class="edit"></td>
8787
<td>
88-
<a class="btn btn-primary btn-sm" href="{{ settings.RFCDIFF_BASE_URL }}?difftype=--hwdiff&amp;url2={{ doc.name }}-{{ doc.rev }}.txt" title="Inline diff (wdiff)">Inline</a>
89-
<a class="btn btn-primary btn-sm" href="{{ settings.RFCDIFF_BASE_URL }}?url2={{ doc.name }}-{{ doc.rev }}.txt" title="Side-by-side diff">Side-by-side</a>
88+
{% include "doc/document_history_form.html" with doc=doc diff_revisions=diff_revisions action=rfcdiff_base_url document_html=document_html only %}
9089
</td>
9190
</tr>
9291
{% endif %}

0 commit comments

Comments
 (0)