Edgewall Software
Modify

Opened 10 years ago

Closed 10 years ago

#9676 closed defect (fixed)

RepositoryIndex macro broken

Reported by: Martin.vGagern@… Owned by: Remy Blank
Priority: high Milestone: 0.12.1
Component: version control/browser Version: 0.12-stable
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

As reported by justink on LP, the RepositoryIndex macro seems to be broken in current trunk.

I've fixed it locally and will attach a suitable patch.

Attachments (3)

LP656638.patch (964 bytes ) - added by Martin.vGagern@… 10 years ago.
Proposed fix
9676-repoindex-macro-r10194.patch (4.8 KB ) - added by Remy Blank 10 years ago.
Multiple fixes to the [[RepositoryIndex()]] macro.
9676-cleandoc-compat-r10210.patch (2.5 KB ) - added by Remy Blank 10 years ago.
Add a fallback for a missing inspect.cleandoc().

Download all attachments as: .zip

Change History (16)

by Martin.vGagern@…, 10 years ago

Attachment: LP656638.patch added

Proposed fix

comment:1 by Remy Blank, 10 years ago

Milestone: 0.12.1
Priority: normalhigh
Version: 0.12-stable

That's a good start, thanks. Unfortunately, the complete fix is a bit more involved than this. I'm working on it.

comment:2 by Christian Boos, 10 years ago

Note that the macro is not robust when a listed repository is invalid, e.g.

Error: Macro RepositoryIndex(None) failed
C:/Workspace/src/trac/plugins/mercurial-plugin-0.12 does not appear to contain a Mercurial repository.
(7460) Trac[Parent 1:formatter] ERROR: Macro RepositoryIndex(None) failed:
Traceback (most recent call last):
  File "C:\Workspace\src\trac\repos\trunk\trac\wiki\formatter.py", line 715, in _macro_formatter
    return macro.process(args, in_paragraph=True)
  File "C:\Workspace\src\trac\repos\trunk\trac\wiki\formatter.py", line 303, in process
    text = self.processor(text)
  File "C:\Workspace\src\trac\repos\trunk\trac\wiki\formatter.py", line 290, in _macro_processor
    text)
  File "C:\Workspace\src\trac\repos\trunk\trac\versioncontrol\web_ui\browser.py", line 879, in expand_macro
    for reponame in all_repos)
  File "C:\Workspace\src\trac\repos\trunk\trac\versioncontrol\web_ui\browser.py", line 879, in <genexpr>
    for reponame in all_repos)
  File "C:\Workspace\src\trac\repos\trunk\trac\versioncontrol\api.py", line 514, in get_repository
    repoinfo.copy())
  File "build\bdist.win32\egg\tracext\hg\backend.py", line 280, in get_repository
    repos = MercurialRepository(dir, options, self.log, self.ui)
  File "build\bdist.win32\egg\tracext\hg\backend.py", line 384, in __init__
    " repository.", path=path))
TracError: C:/Workspace/src/trac/plugins/mercurial-plugin-0.12 does not appear to contain a Mercurial repository.

comment:3 by Remy Blank, 10 years ago

Yeah, I know. There are a few more related issues as well. I'm almost done.

by Remy Blank, 10 years ago

Multiple fixes to the [[RepositoryIndex()]] macro.

comment:4 by Remy Blank, 10 years ago

In addition to the issues fixed by LP656638.patch, 9676-repoindex-macro-r10194.patch fixes multiple other issues with the [[RepositoryIndex()]] macro:

  • Made the macro robust against invalid repositories.
  • Fixed an exception in table format, and restored glob filtering in that format (broken in [10036]).
  • Enabled wiki formatting of changeset messages in table format.
  • Made the desc= argument more robust, and actually use it in compact and list formats.
  • Fixed the column header display (links, ordering) in table format.

Ok to apply?

comment:5 by Christian Boos, 10 years ago

Yes, I tested it and it works fine.

Two further possible tweaks:

  • document desc= arguments (and maybe use a description list for the arguments ''format'' :: ... )
  • in the patch, you simply drop failing repositories; the alternative would be to handle them like the repository index in the source browser and show the error. I'm not sure it's worth doing it, though.

comment:6 by Remy Blank, 10 years ago

I'll add the documentation. About dropping failing repositories, my reasoning was that displaying the error would make the wiki page containing it look ugly, so if you need e.g. to print it, you will get the error as well. The error is visible in the repository index anyway, so I'd rather drop the errors.

I have found another issue, though. If I use an unknown repository type (e.g. git but no plugin installed), I get a 500. Update coming…

comment:7 by Remy Blank, 10 years ago

Resolution: fixed
Status: newclosed

Ok, the 500 is due to something else (authz). I'll move that discussion to #9661.

Patch (with improved, formatted documentation) applied in [10199].

comment:8 by Remy Blank, 10 years ago

Owner: set to Remy Blank

comment:9 by Christian Boos, 10 years ago

Resolution: fixed
Status: closedreopened

I'm seeing this here, in the logs (was with r10198):

Traceback (most recent call last):
  File "build/bdist.linux-x86_64/egg/trac/wiki/formatter.py", line 716, in _macro_formatter
    return macro.process(args, in_paragraph=True)
  File "build/bdist.linux-x86_64/egg/trac/wiki/formatter.py", line 303, in process
    text = self.processor(text)
  File "build/bdist.linux-x86_64/egg/trac/wiki/formatter.py", line 290, in _macro_processor
    text)
  File "build/bdist.linux-x86_64/egg/trac/versioncontrol/web_ui/browser.py", line 880, in expand_macro
    all_repos = sorted((reponame, repos) for reponame, repos in all_repos
  File "build/bdist.linux-x86_64/egg/trac/versioncontrol/web_ui/browser.py", line 880, in <genexpr>
    all_repos = sorted((reponame, repos) for reponame, repos in all_repos
ValueError: too many values to unpack

It seems that this was for accesses to #9661, but the error obviously don't happen all the times. Could it be a transient error due to the hg mirror being synced?

Another, minor issue, is that the help doesn't work for this macro, for some reason: [[RepositoryIndex?()]]

[[RepositoryIndex]]

Display the list of available repositories.

Can be given the following named arguments:

format
Select the rendering format:
  • compact produces a comma-separated list of repository prefix names (default)
  • list produces a description list of repository prefix names
  • table produces a table view, similar to the one visible in the Browse View page
glob
Do a glob-style filtering on the repository names (defaults to '*')
order
Order repositories by the given column (one of "name", "date" or "author")
desc
When set to 1, order by descending order

in reply to:  9 ; comment:10 by Remy Blank, 10 years ago

Replying to cboos:

It seems that this was for accesses to #9661, but the error obviously don't happen all the times. Could it be a transient error due to the hg mirror being synced?

That's the original issue. The fix was only merged to trunk in [10211], and t.e.o is running trunk, right?

Another, minor issue, is that the help doesn't work for this macro, for some reason:

Blast, I forgot to check when inspect.cleandoc() was introduced. Unfortunately it was 2.6. In previous versions, the function was inlined in inspect.getdoc(). So it looks like we will have to have our own copy (maybe named deindent() or dedent()) in trac.util.text.

Working on a fix.

in reply to:  10 comment:11 by Christian Boos, 10 years ago

Replying to rblank:

Replying to cboos:

It seems that this was for accesses to #9661, but the error obviously don't happen all the times. Could it be a transient error due to the hg mirror being synced?

That's the original issue. The fix was only merged to trunk in [10211], and t.e.o is running trunk, right?

My mistake.

Another, minor issue, is that the help doesn't work for this macro, for some reason:

Blast, I forgot to check when inspect.cleandoc() was introduced. Unfortunately it was 2.6. In previous versions, the function was inlined in inspect.getdoc(). So it looks like we will have to have our own copy (maybe named deindent() or dedent()) in trac.util.text.

We also have trunk/trac/util/__init__.py@10218#L465, maybe you can use/enhance that one.

by Remy Blank, 10 years ago

Add a fallback for a missing inspect.cleandoc().

comment:12 by Remy Blank, 10 years ago

9676-cleandoc-compat-r10210.patch adds a fallback implementation if inspect.cleandoc() is missing.

Our get_doc() calls inspect.getdoc(), which already does the de-indentation. I'm only interested in this last step. Of course, a hackish solution would be to put the macro help text into the docstring of get_macro_description(), and to call inspect.getdoc() on the method :)

comment:13 by Remy Blank, 10 years ago

Resolution: fixed
Status: reopenedclosed

Patch applied in [10230], together with a fix to [[MacroList]] to make it list macros alphabetically.

I'll leave the milestone at 0.12.1, as the original issue was fixed there.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.