Summary

ClearCase syntax highlight doesn't work

Review Request #1501 — Created April 2, 2010 and submitted

Information

Owner

jan.koprowski

Repository

Review Board

Branch

master

Bugs

1590

Depends On

Reviewers

Groups

reviewboard

People

Description

After upload diff path in ClearCase version contain part with something similar to @@/main/branch/VERSION. This part is glue after file extension which means it is impossible to pygments determine highlighting by file extension.
This path is proposition to fix the problem.

Testing Done

Change Summary:

Previous soultion based on regular expression can't handle all cases. There was two cases exclusive:
1) /path/to/file@@/main/branch/VERSION
2) /path/to/file/@@/main/branch/VERSION
I can't find one regular expression handle both forms so I decide to solve problem in another way.

Diff:

Revision 2 (+16 -2)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/scmtools/clearcase.py

Change Summary:

Add branch information

Branch:

+	master

reviewboard/diffviewer/diffutils.py (Diff revision 2)

shouldn't this be "ClearCase" ?

jan.koprowski April 9, 2010, 3:41 p.m.

reviewboard/scmtools/clearcase.py:
-------------------------------------
class ClearCaseTool(SCMTool):
    name = 'Clear Case'
-------------------------------------

which means I use value appropriate to hardcoded name in class ClearCaseTool.

djs April 11, 2010, 1:04 p.m.

I see. Looks like the naming is inconsistent between clearcase.py and the scmtool db.

Ship it!

reviewboard/scmtools/clearcase.py (Diff revision 2)

Wrong return parameters. See proper return
line 39, it is a tuple of 2 values

jan.koprowski April 18, 2010, 11:59 p.m.
```
Good notice :) Fixed.
```

Change Summary:

unnextend_path() return two elements tuple even when no revision version.

Diff:

Revision 3 (+16 -2)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/scmtools/clearcase.py

reviewboard/diffviewer/diffutils.py (Diff revision 3)

This could should never know anything about the SCMTools using it.

What I'd recommend is adding a new method to SCMTool for normalizing paths. Maybe call it "normalize_path_for_display".

The method would take a filename and return a result. By default, it would just return the filename. ClearCaseTool could override this to return the result using unextend_path.

jan.koprowski May 15, 2010, 10:01 p.m.

I prepare implementation according Your directions.

Change Summary:

Now there is no "ClearCase" string. New method "normalize_path_for_display" implemented.

Diff:

Revision 4 (+16 -2)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/scmtools/clearcase.py
	reviewboard/scmtools/core.py

Diff:

Revision 5 (+17 -2)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/scmtools/clearcase.py
	reviewboard/scmtools/core.py

Ship it!

Looks good. Committed as 11c95a2. Thanks!