Skip to content

Commit

Permalink
git-webrev: create links for bug ids in commit messages
Browse files Browse the repository at this point in the history
Reviewed-by: rwestberg
  • Loading branch information
edvbld committed Feb 17, 2020
1 parent 64bc771 commit d5b6d4c
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 18 deletions.
22 changes: 16 additions & 6 deletions cli/src/main/java/org/openjdk/skara/cli/GitWebrev.java
Expand Up @@ -83,6 +83,10 @@ private static Hash resolve(ReadOnlyRepository repo, String ref) {
}
}

private static boolean isDigit(char c) {
return Character.isDigit(c);
}

private static void generate(String[] args) throws IOException {
var flags = List.of(
Option.shortcut("r")
Expand Down Expand Up @@ -188,6 +192,7 @@ private static void generate(String[] args) throws IOException {
}
}
}
var upstreamURL = upstream;

var noOutgoing = arguments.contains("no-outgoing");
if (!noOutgoing) {
Expand All @@ -205,12 +210,13 @@ private static void generate(String[] args) throws IOException {
resolve(repo, isMercurial ? "min(outgoing())^" : "origin" + "/" + "master");

var issue = arguments.contains("cr") ? arguments.get("cr").asString() : null;
if (issue != null && !issue.startsWith("http")) {
var digits = Set.of('0', '1', '2', '3', '4', '5', '6', '7', '8', '9');
if (digits.contains(issue.charAt(0))) {
if (issue != null) {
if (issue.startsWith("http")) {
var uri = URI.create(issue);
issue = Path.of(uri.getPath()).getFileName().toString();
} else if (isDigit(issue.charAt(0))) {
issue = "JDK-" + issue;
}
issue = "https://bugs.openjdk.java.net/browse/" + issue;
}
if (issue == null) {
var pattern = Pattern.compile("(?:(JDK|CODETOOLS|JMC|SKARA)-)?([0-9]+).*");
Expand All @@ -224,7 +230,7 @@ private static void generate(String[] args) throws IOException {
project = "JDK";
}
var id = m.group(2);
issue = "https://bugs.openjdk.java.net/browse/" + project + "-" + id;
issue = project + "-" + id;
}
}
}
Expand Down Expand Up @@ -259,6 +265,7 @@ private static void generate(String[] args) throws IOException {
if (username == null) {
username = repo.username().orElse(System.getProperty("user.name"));
}
var author = Author.fromString(username);

if (Files.exists(output)) {
clearDirectory(output);
Expand All @@ -275,11 +282,14 @@ private static void generate(String[] args) throws IOException {
}
}

var jbs = "https://bugs.openjdk.java.net/browse/";
Webrev.repository(repo)
.output(output)
.title(title)
.upstream(upstream)
.username(username)
.username(author.name())
.commitLinker(hash -> upstreamURL == null ? null : upstreamURL + "/commit/" + hash)
.issueLinker(id -> jbs + (isDigit(id.charAt(0)) ? "JDK-" : "") + id)
.issue(issue)
.version(version)
.files(files)
Expand Down
Expand Up @@ -1963,7 +1963,7 @@ void testSubmodulesOnRepoWithSubmodule(VCS vcs) throws IOException {
@ParameterizedTest
@EnumSource(VCS.class)
void testAnnotateTag(VCS vcs) throws IOException {
try (var dir = new TemporaryDirectory()) {
try (var dir = new TemporaryDirectory(false)) {
var repo = Repository.init(dir.path(), vcs);
var readme = repo.root().resolve("README");
var now = ZonedDateTime.now();
Expand Down
47 changes: 39 additions & 8 deletions webrev/src/main/java/org/openjdk/skara/webrev/IndexView.java
Expand Up @@ -57,7 +57,7 @@ class IndexView implements View {

private static final Template UPSTREAM_TEMPLATE = new Template(new String[]{
" <tr>",
" <th>Repository:</th>",
" <th>Compare against:</th>",
" <td><a href=\"${UPSTREAM}\">${UPSTREAM}</a></td>",
" </tr>"
});
Expand All @@ -83,15 +83,28 @@ class IndexView implements View {
" </tr>"
});

private static final Template HEADER_MIDDLE_TEMPLATE = new Template(new String[]{
private static final Template REVISION_TEMPLATE = new Template(new String[]{
" <tr>",
" <th>Compare against revision:</th>",
" <th>Compare against version:</th>",
" <td>${REVISION}</td>",
" </tr>",
" </tr>"
});

private static final Template REVISION_WITH_LINK_TEMPLATE = new Template(new String[]{
" <tr>",
" <th>Compare against version:</th>",
" <td><a href=\"${REVISION_HREF}\">${REVISION}</a></td>",
" </tr>"
});

private static final Template SUMMARY_TEMPLATE = new Template(new String[]{
" <tr>",
" <th>Summary of changes:</th>",
" <td>${STATS}</td>",
" </tr>",
" </tr>"
});

private static final Template PATCH_TEMPLATE = new Template(new String[]{
" <tr>",
" <th>Patch of changes:</th>",
" <td><a href=\"${PATCH_URL}\">${PATCH}</a></td>",
Expand Down Expand Up @@ -139,6 +152,7 @@ public IndexView(List<FileView> files,
String issue,
String version,
Hash revision,
String revisionURL,
Path patchFile,
WebrevStats stats) {
this.files = files;
Expand All @@ -158,7 +172,14 @@ public IndexView(List<FileView> files,

if (pullRequest != null) {
map.put("${PR_HREF}", pullRequest);
map.put("${PR}", pullRequest);

try {
var uri = URI.create(pullRequest);
var id = Path.of(uri.getPath()).getFileName().toString();
map.put("${PR}", id);
} catch (IllegalArgumentException e) {
map.put("${PR}", pullRequest);
}
}


Expand Down Expand Up @@ -187,6 +208,9 @@ public IndexView(List<FileView> files,

map.put("${TITLE}", title);
map.put("${REVISION}", revision.abbreviate());
if (revisionURL != null) {
map.put("${REVISION_HREF}", revisionURL);
}
map.put("${PATCH}", patchFile.toString());
map.put("${PATCH_URL}", patchFile.toString());
map.put("${STATS}", stats.toString());
Expand All @@ -203,17 +227,24 @@ public void render(Writer w) throws IOException {
UPSTREAM_TEMPLATE.render(w, map);
}

if (map.containsKey("${REVISION_HREF}")) {
REVISION_WITH_LINK_TEMPLATE.render(w, map);
} else {
REVISION_TEMPLATE.render(w, map);
}

if (map.containsKey("${BRANCH}")) {
BRANCH_TEMPLATE.render(w, map);
}

HEADER_MIDDLE_TEMPLATE.render(w, map);
SUMMARY_TEMPLATE.render(w, map);
PATCH_TEMPLATE.render(w, map);

if (map.containsKey("${AUTHOR_COMMENT}")) {
AUTHOR_COMMENT_TEMPLATE.render(w, map);
}

if (map.containsKey("${PR}")) {
if (map.containsKey("${PR}") && map.containsKey("${PR_HREF}")) {
PR_TEMPLATE.render(w, map);
}

Expand Down
Expand Up @@ -23,9 +23,27 @@
package org.openjdk.skara.webrev;

import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.Issue;

import java.util.function.Function;

class MetadataFormatter {
private final Function<String, String> issueLinker;

MetadataFormatter(Function<String, String> issueLinker) {
this.issueLinker = issueLinker;
}

String format(CommitMetadata metadata) {
return "<u>" + metadata.hash().abbreviate() +"</u>: " + metadata.message().get(0);
var prefix = metadata.hash().abbreviate() + ": ";
var subject = metadata.message().get(0);
var issue = Issue.fromString(subject);
if (issueLinker != null && issue.isPresent()) {
var id = issue.get().id();
var desc = issue.get().description();
var url = issueLinker.apply(id);
return prefix + "<a href=\"" + url + "\">" + id + "</a>: " + desc;
}
return prefix + subject;
}
}
20 changes: 18 additions & 2 deletions webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java
Expand Up @@ -29,6 +29,7 @@
import java.nio.channels.FileChannel;
import java.nio.file.*;
import java.util.*;
import java.util.function.Function;

import static java.nio.file.StandardOpenOption.*;

Expand Down Expand Up @@ -60,6 +61,8 @@ public static class Builder {
private String pullRequest;
private String branch;
private String issue;
private Function<String, String> issueLinker;
private Function<String, String> commitLinker;
private String version;
private List<Path> files = List.of();

Expand Down Expand Up @@ -98,6 +101,16 @@ public Builder issue(String issue) {
return this;
}

public Builder issueLinker(Function<String, String> issueLinker) {
this.issueLinker = issueLinker;
return this;
}

public Builder commitLinker(Function<String, String> commitLinker) {
this.commitLinker = commitLinker;
return this;
}

public Builder version(String version) {
this.version = version;
return this;
Expand Down Expand Up @@ -174,7 +187,7 @@ public void generate(Hash tailEnd, Hash head) throws IOException {

var headHash = head == null ? repository.head() : head;
var fileViews = new ArrayList<FileView>();
var formatter = new MetadataFormatter();
var formatter = new MetadataFormatter(issueLinker);
for (var patch : patches) {
var status = patch.status();
var path = status.isDeleted() ?
Expand All @@ -194,16 +207,19 @@ public void generate(Hash tailEnd, Hash head) throws IOException {
var total = fileViews.stream().map(FileView::stats).mapToInt(WebrevStats::total).sum();
var stats = new WebrevStats(diff.added(), diff.removed(), diff.modified(), total);

var issueForWebrev = issue == null ? null : issueLinker.apply(issue);
var tailEndURL = commitLinker == null ? null : commitLinker.apply(tailEnd.hex());
try (var w = Files.newBufferedWriter(output.resolve("index.html"))) {
var index = new IndexView(fileViews,
title,
username,
upstream,
branch,
pullRequest,
issue,
issueForWebrev,
version,
tailEnd,
tailEndURL,
output.relativize(patchFile),
stats);
index.render(w);
Expand Down

0 comments on commit d5b6d4c

Please sign in to comment.