diff --git a/reviewboard/diffviewer/diffutils.py b/reviewboard/diffviewer/diffutils.py --- a/reviewboard/diffviewer/diffutils.py +++ b/reviewboard/diffviewer/diffutils.py @@ -1024,6 +1024,7 @@ def get_diff_files(diffset, filediff=None, interdiffset=None, file = { 'depot_filename': filediff.source_file, + 'dest_filename': filediff.dest_file, 'basename': basename, 'basepath': basepath, 'revision': source_revision, @@ -1033,6 +1034,7 @@ def get_diff_files(diffset, filediff=None, interdiffset=None, 'force_interdiff': force_interdiff, 'binary': filediff.binary, 'deleted': filediff.deleted, + 'renamed': filediff.renamed, 'newfile': newfile, 'index': len(files), } diff --git a/reviewboard/diffviewer/forms.py b/reviewboard/diffviewer/forms.py --- a/reviewboard/diffviewer/forms.py +++ b/reviewboard/diffviewer/forms.py @@ -31,6 +31,11 @@ class UploadDiffForm(forms.Form): "This is usually used for distributed revision control " "systems (Git, Mercurial, etc.)."), required=False) + parent_rev = forms.CharField( + label=_("Parent Revision"), + required=False, + help_text=_("The revision the diff is based on. Default is tip."), + widget=forms.TextInput(attrs={'size': '20'})) # Extensions used for intelligent sorting of header files # before implementation files. @@ -45,9 +50,12 @@ class UploadDiffForm(forms.Form): # This SCMTool uses absolute paths, so there's no need to ask # the user for the base directory. del(self.fields['basedir']) + if not self.repository.get_scmtool().get_diffs_use_parent_rev(): + del(self.fields['parent_rev']) - def create(self, diff_file, parent_diff_file=None, diffset_history=None): + def create(self, diff_file, parent_diff_file=None, diffset_history=None, parent_rev=None): tool = self.repository.get_scmtool() + self.parent_rev = parent_rev # Grab the base directory if there is one. if not tool.get_diffs_use_absolute_paths(): @@ -133,7 +141,7 @@ class UploadDiffForm(forms.Form): def _process_files(self, file, basedir, check_existance=False): tool = self.repository.get_scmtool() - for f in tool.get_parser(file.read()).parse(): + for f in tool.get_parser(file.read(), self.parent_rev).parse(): f2, revision = tool.parse_diff_revision(f.origFile, f.origInfo) if f2.startswith("/"): filename = f2 diff --git a/reviewboard/diffviewer/models.py b/reviewboard/diffviewer/models.py --- a/reviewboard/diffviewer/models.py +++ b/reviewboard/diffviewer/models.py @@ -42,6 +42,11 @@ class FileDiff(models.Model): def deleted(self): return self.status == 'D' + @property + def renamed(self): + return ((not (self.source_file == self.dest_file)) and + (len(self.diff) == 0)) + def __unicode__(self): return u"%s (%s) -> %s (%s)" % (self.source_file, self.source_revision, self.dest_file, self.dest_detail) diff --git a/reviewboard/reviews/forms.py b/reviewboard/reviews/forms.py --- a/reviewboard/reviews/forms.py +++ b/reviewboard/reviews/forms.py @@ -55,7 +55,7 @@ class DefaultReviewerForm(forms.ModelForm): class NewReviewRequestForm(forms.Form): """ - A form that handles creationg of new review requests. These take + A form that handles creating of new review requests. These take information on the diffs, the repository the diffs are against, and optionally a changelist number (for use in certain repository types such as Perforce). @@ -88,6 +88,12 @@ class NewReviewRequestForm(forms.Form): changenum = forms.IntegerField(label=_("Change Number"), required=False) + parent_rev = forms.CharField( + label=_("Parent Revision"), + required=False, + help_text=_("The revision the diff is based on. Default is tip."), + widget=forms.TextInput(attrs={'size': '20'})) + field_mapping = {} def __init__(self, *args, **kwargs): @@ -130,6 +136,7 @@ class NewReviewRequestForm(forms.Form): def create(self, user, diff_file, parent_diff_file): repository = self.cleaned_data['repository'] changenum = self.cleaned_data['changenum'] or None + parent_rev = self.cleaned_data['parent_rev'] or None # It's a little odd to validate this here, but we want to have access to # the user. @@ -156,6 +163,25 @@ class NewReviewRequestForm(forms.Form): self.errors['changenum'] = forms.util.ErrorList([ 'This change number is owned by another user.']) raise OwnershipError() + if parent_rev: + changeset2 = None + try: + changeset2 = repository.get_scmtool().get_changeset(parent_rev) + except NotImplementedError: + # This scmtool doesn't have changesets + pass + except SCMError, e: + self.errors['parent_rev'] = forms.util.ErrorList([str(e)]) + raise ChangeSetError() + except ChangeSetError, e: + self.errors['parent_rev'] = forms.util.ErrorList([str(e)]) + raise e + + if not changeset2: + self.errors['parent_rev'] = forms.util.ErrorList([ + 'This revision does not represent a valid ' + 'changeset.']) + raise InvalidChangeNumberError() try: review_request = ReviewRequest.objects.create(user, repository, @@ -194,7 +220,7 @@ class NewReviewRequestForm(forms.Form): try: diff_form.create(diff_file, parent_diff_file, - attach_to_history=True) + attach_to_history=True, parent_rev=parent_rev) if 'path' in diff_form.errors: self.errors['diff_path'] = diff_form.errors['path'] raise SavedError @@ -238,7 +264,7 @@ class UploadDiffForm(diffviewer_forms.UploadDiffForm): pass def create(self, diff_file, parent_diff_file=None, - attach_to_history=False): + attach_to_history=False, parent_rev=None): history = None if attach_to_history: @@ -246,7 +272,7 @@ class UploadDiffForm(diffviewer_forms.UploadDiffForm): diffset = super(UploadDiffForm, self).create(diff_file, parent_diff_file, - history) + history, parent_rev) if not attach_to_history: # Set the initial revision to be one newer than the most recent diff --git a/reviewboard/reviews/views.py b/reviewboard/reviews/views.py --- a/reviewboard/reviews/views.py +++ b/reviewboard/reviews/views.py @@ -520,6 +520,7 @@ def build_diff_comment_fragments( 'depot_filename': comment.filediff.source_file, 'index': None, 'filediff': comment.filediff, + 'dest_filename': comment.filediff.dest_file }, }) diff --git a/reviewboard/scmtools/core.py b/reviewboard/scmtools/core.py --- a/reviewboard/scmtools/core.py +++ b/reviewboard/scmtools/core.py @@ -76,6 +76,9 @@ class SCMTool(object): def get_diffs_use_absolute_paths(self): return False + def get_diffs_use_parent_rev(self): + return False + def get_changeset(self, changesetid): raise NotImplementedError @@ -94,7 +97,7 @@ class SCMTool(object): # review request" page. raise NotImplementedError - def get_parser(self, data): + def get_parser(self, data, def_rev = None): return diffparser.DiffParser(data) @classmethod diff --git a/reviewboard/scmtools/git.py b/reviewboard/scmtools/git.py --- a/reviewboard/scmtools/git.py +++ b/reviewboard/scmtools/git.py @@ -73,7 +73,7 @@ class GitTool(SCMTool): def get_fields(self): return ['diff_path'] - def get_parser(self, data): + def get_parser(self, data, def_rev=None): return GitDiffParser(data) @classmethod @@ -105,6 +105,11 @@ class GitDiffParser(DiffParser): """ pre_creation_regexp = re.compile("^0+$") + def __init__(self, data, def_rev=None): + self.data = data + self.lines = data.splitlines() + self.def_rev = def_rev + def parse(self): """ Parses the diff, returning a list of File objects representing each @@ -147,8 +152,13 @@ class GitDiffParser(DiffParser): file_info = File() file_info.data = self.lines[linenum] + "\n" file_info.binary = False + if self.def_rev: + file_info.origInfo = self.def_rev + file_info.newInfo = "Uncommitted" + file_info.origChangesetId = self.def_rev diff_line = self.lines[linenum].split() - + if self.lines[linenum + 1].startswith("deleted file mode"): + file_info.deleted = True try: # Need to remove the "a/" and "b/" prefix file_info.origFile = GIT_DIFF_PREFIX.sub("", diff_line[-2]) @@ -163,7 +173,13 @@ class GitDiffParser(DiffParser): linenum += 1 elif self._is_mode_change(linenum): linenum += 2 - + elif self._is_renamed_change(linenum): + file_info.origFile = self.lines[linenum].split()[-1] + file_info.newFile = self.lines[linenum + 1].split()[-1] + linenum += 2 + if ((linenum) >= len(self.lines) or self.lines[linenum].startswith("diff --git")): + file_info.data = None + return linenum, file_info if self._is_index_range_line(linenum): index_range = self.lines[linenum].split(None, 2)[1] @@ -194,12 +210,15 @@ class GitDiffParser(DiffParser): return linenum, file_info def _is_empty_change(self, linenum): - next_diff_start = self.lines[linenum + GIT_DIFF_EMPTY_CHANGESET_SIZE] + if len(self.lines) > linenum + GIT_DIFF_EMPTY_CHANGESET_SIZE: + next_diff_start = self.lines[linenum + GIT_DIFF_EMPTY_CHANGESET_SIZE] + else: + next_diff_start = None next_line = self.lines[linenum + 1] return ((next_line.startswith("new file mode") or next_line.startswith("old mode") or next_line.startswith("deleted file mode")) - and next_diff_start.startswith("diff --git")) + and (not next_diff_start or next_diff_start.startswith("diff --git"))) def _is_newfile_or_deleted_change(self, linenum): line = self.lines[linenum] @@ -207,6 +226,10 @@ class GitDiffParser(DiffParser): return (line.startswith("new file mode") or line.startswith("deleted file mode")) + def _is_renamed_change(self, linenum): + return (self.lines[linenum].startswith("rename from") + and self.lines[linenum + 1].startswith("rename to")) + def _is_mode_change(self, linenum): return (self.lines[linenum].startswith("old mode") and self.lines[linenum + 1].startswith("new mode")) diff --git a/reviewboard/scmtools/hg.py b/reviewboard/scmtools/hg.py --- a/reviewboard/scmtools/hg.py +++ b/reviewboard/scmtools/hg.py @@ -9,7 +9,7 @@ except ImportError: from reviewboard.diffviewer.parser import DiffParser, DiffParserError from reviewboard.scmtools.git import GitDiffParser from reviewboard.scmtools.core import \ - FileNotFoundError, SCMTool, HEAD, PRE_CREATION + FileNotFoundError, SCMTool, ChangeSet, HEAD, PRE_CREATION class HgTool(SCMTool): @@ -43,12 +43,33 @@ class HgTool(SCMTool): def get_diffs_use_absolute_paths(self): return True + def get_diffs_use_parent_rev(self): + return True + + def get_changeset(self, changesetid): + # This is used mostly for checking the changeset exists so not + # returning much real data in the ChangeSet structure + rev = None + try: + rev = self.client.get_rev_id(changesetid) + except Exception: + pass + if not rev: + return None + changeset = ChangeSet() + changeset.changenum = rev + return changeset + def get_fields(self): - return ['diff_path'] + return ['diff_path', 'parent_rev'] - def get_parser(self, data): + def get_parser(self, data, def_rev="tip"): + if not def_rev: + rev = self.client.get_rev_id("tip") + else: + rev = self.client.get_rev_id(def_rev) if data.lstrip().startswith('diff --git'): - return GitDiffParser(data) + return GitDiffParser(data, rev) else: return HgDiffParser(data) @@ -100,6 +121,8 @@ class HgDiffParser(DiffParser): # check if we're a new file if self.lines[linenum].split()[1] == "/dev/null": info['origInfo'] = PRE_CREATION + if self.lines[linenum + 1].split()[1] == "/dev/null": + info['deleted'] = True linenum += 2; return linenum @@ -108,11 +131,27 @@ class HgWebClient(object): FULL_FILE_URL = '%(url)s/%(rawpath)s/%(revision)s/%(quoted_path)s' def __init__(self, repoPath, username, password): + try: + from mercurial import hg, ui, node + from mercurial.__version__ import version + + version_parts = [int(x) for x in version.split(".")] + + if version_parts[0] == 1 and version_parts[1] <= 2: + hg_ui = ui.ui(interactive=False) + else: + hg_ui = ui.ui() + hg_ui.setconfig('ui', 'interactive', 'off') + + self.repo = hg.repository(hg_ui, path=repoPath) + except ImportError: + self.repo = None + self.url = repoPath self.username = username self.password = password logging.debug('Initialized HgWebClient with url=%r, username=%r', - self.url, self.username) + self.url, self.username) def cat_file(self, path, rev="tip"): if rev == HEAD: @@ -157,6 +196,12 @@ class HgWebClient(object): def get_filenames(self, rev): raise NotImplemented + def get_rev_id(self, rev="tip"): + import binascii + if self.repo: + return binascii.hexlify(self.repo.lookup(rev))[:12] + else: + return None class HgClient(object): @@ -189,3 +234,6 @@ class HgClient(object): def get_filenames(self, rev): return self.repo.changectx(rev).TODO + + def get_rev_id(self, rev="tip"): + return self.repo.changectx(rev).hex()[:12] diff --git a/reviewboard/scmtools/tests.py b/reviewboard/scmtools/tests.py --- a/reviewboard/scmtools/tests.py +++ b/reviewboard/scmtools/tests.py @@ -639,13 +639,13 @@ class MercurialTests(DjangoTestCase): """Testing basic HgTool API""" self.assert_(self.tool.get_diffs_use_absolute_paths()) - self.assertRaises(NotImplementedError, - lambda: self.tool.get_changeset(1)) + desc = self.tool.get_changeset('661e5dd3c493') + self.assertEqual(desc.changenum, '661e5dd3c493') self.assertRaises(NotImplementedError, lambda: self.tool.get_pending_changesets(1)) - self.assertEqual(self.tool.get_fields(), ['diff_path']) + self.assertEqual(self.tool.get_fields(), ['diff_path', 'parent_rev']) class GitTests(DjangoTestCase): diff --git a/reviewboard/templates/diffviewer/changeindex.html b/reviewboard/templates/diffviewer/changeindex.html --- a/reviewboard/templates/diffviewer/changeindex.html +++ b/reviewboard/templates/diffviewer/changeindex.html @@ -1,7 +1,13 @@ {% load i18n %}
    {% for file in files %} -
  1. {{file.depot_filename}}: +
  2. + {% ifequal file.depot_filename file.dest_filename %} + {{file.depot_filename}} + {% else %} + {{file.dest_filename}} (was {{file.depot_filename}}) + {% endifequal %} + : {% trans
    3. diff --git a/reviewboard/templates/diffviewer/changeindex_entry.html b/reviewboard/templates/diffviewer/changeindex_entry.html --- a/reviewboard/templates/diffviewer/changeindex_entry.html +++ b/reviewboard/templates/diffviewer/changeindex_entry.html @@ -1,7 +1,8 @@ {% load i18n %} -{{file.depot_filename}}: +{{file.dest_filename}}: {% if error %} {% trans "Diff currently unavailable." %} +{{error}} {% else %} {% if file.binary %} {% trans "binary file" %} @@ -9,6 +10,10 @@ {% if file.deleted %} {% trans "deleted" %} {% else %} +{% if file.renamed %} +{% trans "renamed from " %} +{{file.depot_filename}} +{% else %} {% blocktrans count file.num_changes as counter %} 1 change {% plural %} @@ -22,6 +27,7 @@ {% endfor %} {% endifequal %} ] +{% endif %} {# !renamed #} {% endif %}{# !deleted #} {% endif %}{# !binary #} {% endif %}{# !error #} diff --git a/reviewboard/templates/diffviewer/diff_file_fragment.html b/reviewboard/templates/diffviewer/diff_file_fragment.html --- a/reviewboard/templates/diffviewer/diff_file_fragment.html +++ b/reviewboard/templates/diffviewer/diff_file_fragment.html @@ -6,7 +6,7 @@ {{error}} {% endif %} -{% if file.changed_chunk_indexes or file.binary or file.deleted %} +{% if file.changed_chunk_indexes or file.binary or file.deleted or file.renamed %} {% if not standalone %} @@ -17,7 +17,11 @@ + {% ifequal file.depot_filename file.dest_filename %} + {% else %} + + {% endifequal %} @@ -25,6 +29,13 @@ {% endif %}{# not standalone #} +{% if file.renamed %} + + + + + +{% else %} {% if file.binary %} @@ -98,6 +109,7 @@ {% endfor %}{# chunks #} {% endif %}{# not file.deleted #} {% endif %}{# not file.binary #} +{% endif %}{# not file.renamed #} {% if not standalone %}
    {{ file.depot_filename }}{{ file.dest_filename }} (was {{ file.depot_filename }})
    {{file.revision}}
    {% trans "This file was only renamed. The content will not be displayed." %}