patx/hglab
improve ref selector and tests for pull requests
Commit 59d9840e2fe7 · Harrison Erd · 2026-05-05 02:03 -0400
Comments
No comments yet.
Diff
diff --git a/app.py b/app.py
--- a/app.py
+++ b/app.py
@@ -67,9 +67,10 @@
REV_RE = re.compile(r"^(null|[0-9a-fA-F]{1,40})$")
REF_TYPE_BRANCH = "branch"
REF_TYPE_BOOKMARK = "bookmark"
+REF_TYPE_TAG = "tag"
REF_TYPE_TIP = "tip"
REF_TYPE_COMMIT = "commit"
-REF_TYPES = {REF_TYPE_BRANCH, REF_TYPE_BOOKMARK, REF_TYPE_TIP, REF_TYPE_COMMIT}
+REF_TYPES = {REF_TYPE_BRANCH, REF_TYPE_BOOKMARK, REF_TYPE_TAG, REF_TYPE_TIP, REF_TYPE_COMMIT}
PULL_REQUEST_REF_TYPES = {REF_TYPE_BRANCH, REF_TYPE_BOOKMARK, REF_TYPE_TIP}
REF_QUERY_KEYS = {"ref", "ref_type", "ref_value"}
REF_VALUE_SEPARATOR = "|"
@@ -1485,11 +1486,17 @@
continue
tags.append(
{
+ "type": REF_TYPE_TAG,
"name": parts[0],
+ "label": f"tag {parts[0]}",
"shell_name": shlex.quote(parts[0]),
"rev": parts[1],
"node": parts[2],
"short_node": parts[3],
+ "branch": "",
+ "active": False,
+ "closed": False,
+ "is_default": False,
"date": parts[4],
"summary": parts[5],
}
@@ -1682,6 +1689,11 @@
if bookmark["name"] == ref_name:
return dict(bookmark)
raise ValueError("Bookmark not found.")
+ if ref_type == REF_TYPE_TAG:
+ for tag in list_repo_tags(path):
+ if tag["name"] == ref_name:
+ return dict(tag)
+ raise ValueError("Tag not found.")
raise ValueError("Ref not found.")
@@ -1768,6 +1780,8 @@
return f"branch {ref_name}"
if ref_type == REF_TYPE_BOOKMARK:
return f"bookmark {ref_name}"
+ if ref_type == REF_TYPE_TAG:
+ return f"tag {ref_name}"
if ref_type == REF_TYPE_COMMIT:
return f"commit {ref_name[:12]}" if ref_name else "commit"
return "tip"
@@ -1792,23 +1806,33 @@
}
-def repo_ref_options(path, include_closed_branches=True, include_tip=True):
+def repo_ref_options(path, include_closed_branches=True, include_tip=True, include_tags=True):
branches = list_repo_branches(path)
bookmarks = list_repo_bookmarks(path)
- options = []
+ refs = []
for branch in branches:
if include_closed_branches or not branch["closed"]:
- options.append(ref_option_from_ref(branch))
- for bookmark in bookmarks:
- options.append(ref_option_from_ref(bookmark))
+ refs.append(branch)
+ refs.extend(bookmarks)
+ if include_tags:
+ refs.extend(list_repo_tags(path))
+
+ refs.sort(key=newest_revision_sort_key, reverse=True)
+ options = []
+ for index, ref in enumerate(refs):
+ option = ref_option_from_ref(ref)
+ option["is_initial"] = index < 10
+ options.append(option)
if include_tip:
- options.append(ref_option_from_ref(tip_ref(path)))
+ tip_option = ref_option_from_ref(tip_ref(path))
+ tip_option["is_initial"] = False
+ options.append(tip_option)
return options
def source_repo_ref_options(source_repo):
path = repo_path(source_repo["owner_username"], source_repo["name"])
- options = repo_ref_options(path, include_closed_branches=True, include_tip=True)
+ options = repo_ref_options(path, include_closed_branches=True, include_tip=True, include_tags=False)
for option in options:
option["value"] = source_ref_option_value(
source_repo["id"],
@@ -1820,7 +1844,7 @@
def target_repo_ref_options(path):
- return repo_ref_options(path, include_closed_branches=False, include_tip=True)
+ return repo_ref_options(path, include_closed_branches=False, include_tip=True, include_tags=False)
def revision_branch(path, node):
diff --git a/templates/base.tpl b/templates/base.tpl
--- a/templates/base.tpl
+++ b/templates/base.tpl
@@ -48,15 +48,29 @@
const pickers = document.querySelectorAll("[data-ref-picker]");
if (!pickers.length) return;
- const resetFilter = (picker) => {
+ const applyFilter = (picker) => {
const search = picker.querySelector("[data-ref-picker-search]");
const options = picker.querySelectorAll("[data-ref-picker-option]");
const empty = picker.querySelector("[data-ref-picker-empty]");
- if (search) search.value = "";
+ const query = search ? search.value.trim().toLowerCase() : "";
+ let visibleCount = 0;
+
options.forEach((option) => {
- option.hidden = false;
+ const label = option.dataset.refLabel || "";
+ const isVisible = query
+ ? label.includes(query)
+ : option.dataset.refInitial === "true";
+ option.hidden = !isVisible;
+ if (isVisible) visibleCount += 1;
});
- if (empty) empty.hidden = true;
+
+ if (empty) empty.hidden = visibleCount > 0;
+ };
+
+ const resetFilter = (picker) => {
+ const search = picker.querySelector("[data-ref-picker-search]");
+ if (search) search.value = "";
+ applyFilter(picker);
};
const closePicker = (picker) => {
@@ -84,8 +98,6 @@
pickers.forEach((picker) => {
const button = picker.querySelector(".ref-picker-toggle");
const search = picker.querySelector("[data-ref-picker-search]");
- const options = picker.querySelectorAll("[data-ref-picker-option]");
- const empty = picker.querySelector("[data-ref-picker-empty]");
if (button) {
button.addEventListener("click", () => {
@@ -100,14 +112,7 @@
if (search) {
search.addEventListener("input", () => {
- const query = search.value.trim().toLowerCase();
- let visibleCount = 0;
- options.forEach((option) => {
- const isVisible = option.dataset.refLabel.includes(query);
- option.hidden = !isVisible;
- if (isVisible) visibleCount += 1;
- });
- if (empty) empty.hidden = visibleCount > 0;
+ applyFilter(picker);
});
}
});
diff --git a/templates/ref_selector.tpl b/templates/ref_selector.tpl
--- a/templates/ref_selector.tpl
+++ b/templates/ref_selector.tpl
@@ -13,7 +13,15 @@
<div class="ref-picker-options" role="menu">
% for option in ref_options:
% is_selected = option["value"] == selected_ref_value
- <a class="ref-picker-option {{'active' if is_selected else ''}}" href="{{current_url_with_ref(option['ref'])}}" data-ref-picker-option data-ref-label="{{option['label'].lower()}}" role="menuitem" aria-current="{{'page' if is_selected else 'false'}}">
+ <a
+ class="ref-picker-option {{'active' if is_selected else ''}}"
+ href="{{current_url_with_ref(option['ref'])}}"
+ data-ref-picker-option
+ data-ref-label="{{option['label'].lower()}}"
+ data-ref-initial="{{'true' if option.get('is_initial') or is_selected else 'false'}}"
+ role="menuitem"
+ aria-current="{{'page' if is_selected else 'false'}}"
+ >
<span>{{option["label"]}}</span>
% if is_selected:
<span class="ref-picker-current">current</span>
diff --git a/tests/test_app.py b/tests/test_app.py
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -430,6 +430,12 @@
assert value == "branch|feature%2Fa%7Cb%20c"
assert hglab.parse_ref_option_value(value) == (hglab.REF_TYPE_BRANCH, branch_name)
+ tag_name = "release/a|b c"
+ tag_value = hglab.ref_option_value(hglab.REF_TYPE_TAG, tag_name)
+
+ assert tag_value == "tag|release%2Fa%7Cb%20c"
+ assert hglab.parse_ref_option_value(tag_value) == (hglab.REF_TYPE_TAG, tag_name)
+
def test_source_ref_option_values_round_trip_and_validate_source_repo():
branch_name = "feature/a|b"
@@ -457,6 +463,7 @@
def test_format_ref_label_and_closed_branch_option_label():
assert hglab.format_ref_label(hglab.REF_TYPE_TIP) == "tip"
+ assert hglab.format_ref_label(hglab.REF_TYPE_TAG, "v1.0") == "tag v1.0"
assert hglab.format_ref_label(hglab.REF_TYPE_COMMIT, "abcdef1234567890") == "commit abcdef123456"
assert hglab.ref_option_label({"type": hglab.REF_TYPE_BRANCH, "name": "old", "closed": True}) == (
"branch old (closed)"
@@ -719,6 +726,76 @@
assert "new feature" in diff
+def test_pull_request_source_refs_can_use_fork_branches_and_bookmarks(isolated_app):
+ owner = create_user("alice")
+ author = create_user("bob")
+ isolated_app.create_repository(owner, "demo", "")
+ target_path = isolated_app.repo_path("alice", "demo")
+ commit_file(target_path, "README.md", "# Demo\n", message="initial", user="alice")
+ target_repo = isolated_app.get_repo("alice", "demo")
+
+ isolated_app.fork_repository(author, target_repo, "demo-fork", "forked copy")
+ source_repo = isolated_app.get_repo("bob", "demo-fork")
+ source_path = isolated_app.repo_path("bob", "demo-fork")
+ isolated_app.run_hg(["branch", "feature/pr"], cwd=source_path)
+ branch_node = commit_file(
+ source_path,
+ "feature.txt",
+ "branch feature\n",
+ message="branch feature",
+ user="bob",
+ )
+ isolated_app.run_hg(["bookmark", "-r", branch_node, "feature-bm"], cwd=source_path)
+
+ source_labels = [option["label"] for option in isolated_app.source_repo_ref_options(source_repo)]
+
+ assert "bob/demo-fork branch feature/pr" in source_labels
+ assert "bob/demo-fork bookmark feature-bm" in source_labels
+ assert not any(" tag " in label for label in source_labels)
+
+ branch_pr_number = isolated_app.create_pull_request(
+ target_repo,
+ source_repo,
+ author,
+ "Branch PR",
+ "",
+ isolated_app.REF_TYPE_BRANCH,
+ "feature/pr",
+ isolated_app.REF_TYPE_TIP,
+ "",
+ )
+ branch_pr = isolated_app.get_pull_request(target_repo["id"], branch_pr_number)
+ branch_diff, branch_source_node, branch_source_ref = isolated_app.pull_request_diff(branch_pr)
+
+ assert branch_pr["source_ref_type"] == isolated_app.REF_TYPE_BRANCH
+ assert branch_pr["source_ref_name"] == "feature/pr"
+ assert branch_pr["source_node"] == branch_node
+ assert branch_source_node == branch_node
+ assert branch_source_ref["type"] == isolated_app.REF_TYPE_BRANCH
+ assert "branch feature" in branch_diff
+
+ bookmark_pr_number = isolated_app.create_pull_request(
+ target_repo,
+ source_repo,
+ author,
+ "Bookmark PR",
+ "",
+ isolated_app.REF_TYPE_BOOKMARK,
+ "feature-bm",
+ isolated_app.REF_TYPE_TIP,
+ "",
+ )
+ bookmark_pr = isolated_app.get_pull_request(target_repo["id"], bookmark_pr_number)
+ bookmark_diff, bookmark_source_node, bookmark_source_ref = isolated_app.pull_request_diff(bookmark_pr)
+
+ assert bookmark_pr["source_ref_type"] == isolated_app.REF_TYPE_BOOKMARK
+ assert bookmark_pr["source_ref_name"] == "feature-bm"
+ assert bookmark_pr["source_node"] == branch_node
+ assert bookmark_source_node == branch_node
+ assert bookmark_source_ref["type"] == isolated_app.REF_TYPE_BOOKMARK
+ assert "branch feature" in bookmark_diff
+
+
def test_branch_tag_bookmark_helpers_resolve_and_filter_refs(isolated_app):
owner = create_user("alice")
nodes = create_repo_with_refs(owner)
@@ -729,22 +806,51 @@
bookmarks = {bookmark["name"]: bookmark for bookmark in isolated_app.list_repo_bookmarks(path)}
target_labels = [option["label"] for option in isolated_app.target_repo_ref_options(path)]
all_labels = [option["label"] for option in isolated_app.repo_ref_options(path)]
+ source_labels = [
+ option["label"] for option in isolated_app.source_repo_ref_options(isolated_app.get_repo("alice", "demo"))
+ ]
assert {"default", "feature", "old"}.issubset(branches)
assert branches["default"]["node"] == nodes["default_node"]
assert branches["feature"]["closed"] is False
assert branches["old"]["closed"] is True
assert tags[0]["name"] == "v1.0"
+ assert tags[0]["type"] == isolated_app.REF_TYPE_TAG
assert tags[0]["node"] == nodes["feature_node"]
assert bookmarks["feature-bm"]["node"] == nodes["feature_node"]
assert isolated_app.default_code_ref(path)["node"] == nodes["default_node"]
assert isolated_app.resolve_repo_ref(path, isolated_app.REF_TYPE_BRANCH, "feature")["name"] == "feature"
+ assert isolated_app.resolve_repo_ref(path, isolated_app.REF_TYPE_TAG, "v1.0")["node"] == nodes["feature_node"]
assert isolated_app.resolve_repo_ref(path, isolated_app.REF_TYPE_BOOKMARK, "feature-bm")["node"] == (
nodes["feature_node"]
)
assert isolated_app.commit_ref(path, nodes["feature_node"])["type"] == isolated_app.REF_TYPE_COMMIT
+ assert "tag v1.0" in all_labels
assert "branch old (closed)" not in target_labels
+ assert "tag v1.0" not in target_labels
assert "branch old (closed)" in all_labels
+ assert "alice/demo tag v1.0" not in source_labels
+
+
+def test_repo_ref_options_mark_ten_newest_named_refs_for_picker(isolated_app):
+ owner = create_user("alice")
+ isolated_app.create_repository(owner, "many", "")
+ path = isolated_app.repo_path("alice", "many")
+ node = commit_file(path, "README.md", "# Many\n", message="initial", user=owner["username"])
+ for index in range(12):
+ isolated_app.run_hg(["bookmark", "-r", node, f"bm-{index:02d}"], cwd=path)
+
+ options = isolated_app.repo_ref_options(path)
+ named_options = [option for option in options if option["ref"]["type"] != isolated_app.REF_TYPE_TIP]
+ initial_options = [option for option in named_options if option["is_initial"]]
+ named_revs = [int(option["ref"]["rev"]) for option in named_options]
+
+ assert len(named_options) == 13
+ assert len(initial_options) == 10
+ assert named_revs == sorted(named_revs, reverse=True)
+ assert not any(
+ option["is_initial"] for option in options if option["ref"]["type"] == isolated_app.REF_TYPE_TIP
+ )
def test_bottle_signup_login_logout_and_new_repo_flow(isolated_app):
@@ -809,6 +915,8 @@
("/alice/demo/bookmarks", 200, "feature-bm"),
("/alice/demo/src?ref_type=branch&ref=feature", 200, "feature.txt"),
("/alice/demo/src/feature.txt?ref_type=branch&ref=feature", 200, "feature work"),
+ ("/alice/demo/src?ref_type=tag&ref=v1.0", 200, "feature.txt"),
+ ("/alice/demo/commits?ref_type=tag&ref=v1.0", 200, "add feature"),
("/alice/demo/issues", 200, "No open issues."),
("/alice/demo/pulls", 200, "No open pull requests."),
("/static/icon.png", 200, ""),
@@ -817,6 +925,7 @@
("/alice/demo/src/missing.txt", 404, "Path not found."),
("/alice/demo/src/docs/../secret", 400, "Invalid repository path."),
("/alice/demo/src?ref_type=branch&ref=missing", 404, "Branch not found."),
+ ("/alice/demo/src?ref_type=tag&ref=missing", 404, "Tag not found."),
]
for path, status_code, expected_text in checks:
@@ -824,6 +933,10 @@
assert response.status_code == status_code, path
assert expected_text in response.text, path
+ repo_response = client.get("/alice/demo")
+ assert 'data-ref-label="tag v1.0"' in repo_response.text
+ assert 'data-ref-initial="true"' in repo_response.text
+
response = client.get("/alice/demo/raw/feature.txt?ref_type=branch&ref=feature")
assert response.status_code == 200
assert response.body == b"feature work\n"
@@ -932,7 +1045,9 @@
isolated_app.fork_repository(author, target_repo, "demo-fork", "forked copy")
source_repo = isolated_app.get_repo("bob", "demo-fork")
source_path = isolated_app.repo_path("bob", "demo-fork")
+ isolated_app.run_hg(["branch", "feature/pr"], cwd=source_path)
source_node = commit_file(source_path, "feature.txt", "new feature\n", message="add feature", user="bob")
+ isolated_app.run_hg(["bookmark", "-r", source_node, "feature-bm"], cwd=source_path)
bob_client = WsgiClient(isolated_app.app)
login_client(bob_client, "bob")
@@ -940,11 +1055,15 @@
response = bob_client.get("/alice/demo/pulls/new")
assert response.status_code == 200
assert "bob/demo-fork tip" in response.text
+ assert "bob/demo-fork branch feature/pr" in response.text
+ assert "bob/demo-fork bookmark feature-bm" in response.text
response = bob_client.post(
"/alice/demo/pulls/new",
{
- "source_ref": isolated_app.source_ref_option_value(source_repo["id"], isolated_app.REF_TYPE_TIP, ""),
+ "source_ref": isolated_app.source_ref_option_value(
+ source_repo["id"], isolated_app.REF_TYPE_BRANCH, "feature/pr"
+ ),
"target_ref": isolated_app.ref_option_value(isolated_app.REF_TYPE_TIP, ""),
"title": "Add feature",
"body": "Please merge this",
@@ -956,6 +1075,8 @@
pr = isolated_app.get_pull_request(target_repo["id"], 1)
assert pr["base_node"] == base_node
assert pr["source_node"] == source_node
+ assert pr["source_ref_type"] == isolated_app.REF_TYPE_BRANCH
+ assert pr["source_ref_name"] == "feature/pr"
response = bob_client.get("/alice/demo/pulls/1")
assert response.status_code == 200