Edgewall Software
Modify

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#8536 closed defect (fixed)

Don't display Default Repository on main Browse page

Reported by: John Hampton Owned by: Remy Blank
Priority: normal Milestone: 0.12-multirepos
Component: version control Version: none
Severity: normal Keywords: multirepos
Cc:
Release Notes:
API Changes:

Description

When using the multirepos branch, the "Browse" link shows both a list of the configured repositories, along with the contents of the "default" repository below.

This is ugly and a touch confusing. If I have many repositories configured, then when visiting "Browse", I expect to see a list of the repositories. Not a list of the repositories, and then one of those repos below. At best, showing the default repository clutters the page.

Now, I understand that some crazy person may find this useful. So, perhaps a config setting show_default_repos with it disabled by default is the best answer.

Attachments (2)

8536-hide-repos-r8439.patch (5.4 KB ) - added by Remy Blank 8 years ago.
Add possibility to hide repositories from the index, and clean up index.
8536-hide-repos-r8441.patch (9.0 KB ) - added by Remy Blank 8 years ago.
Improved patch fixing aliases and .hidden values.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 8 years ago by Remy Blank

Milestone: 0.12
Owner: set to Remy Blank

The default repository display is useful for single-repository projects. Most multi-repository projects will indeed want to have it hidden, but still active to avoid breaking links created before multirepos. So a config option is certainly the best option. I'm not sure whether it should be enabled or disabled by default, though.

comment:2 Changed 8 years ago by Christian Boos

Summary: Don't display Defautl repository on main Browse pageDon't display Default Repository on main Browse page

The real question here is at which URL would you display the toplevel content of the default repository? /browser/(default)? That seems also ugly to me and would introduce special cases in many different places.

There's also the "legacy" aspect, by which people used to access their main repository by clicking on Browse Source would be annoyed by having to go systematically one more level down. I suspect those people would even rather see the default repository first, then the list of repositories next.

(Note: remove "If this is a problem, add a separate explicit entry for that repository.", as I can't figure anymore what I meant with this…)

comment:3 Changed 8 years ago by Remy Blank

For me, there are three situations:

  • Single-repository installations: in this case, you only want to see the default repository, without having to dig one additional level.
  • Multi-repository installations upgraded from a 0.11 single-repository installations: in this case, to keep all previous source: links consistent, you will want to keep the default repository active, but invisible in the browser, and add an alias for it with a name. Or maybe the other way around: give a name to the default repository, and make the default an alias for that. From that point, you create source: links with the named repository.
  • New multi-repository installations: here you don't even want a default repository, so you remove it altogether.

So, the only option we need is to be allowed to have a default repository, accessible at /browser/... as it is now, but invisible in the repository index page /browser.

About the order, I agree that the default repository should be shown first if it is visible.

comment:4 Changed 8 years ago by Christian Boos

I'd rather label the three categories as:

single repository
Show only the default repository, not the index
Indeed, will do.
multi-repository with default repository
always have an alias for the default repository
If not defined explicitly, we could take the basename of the repository dir; this would also remove the need for a special (default) entry, but we could still display the entry for the alias of the default repository in bold.
multi-repository without default repository
only show repository index
This is already the current behavior.

I think the second case can also happen for new multi-repository environments, as there are situations where you'd want to have a default repository defined: a main repository where the code is, and a few ancillary repositories less frequently used but that need to be accessible as well (e.g. legacy repositories). In this use case, I believe it make sense to display both the default repository (first) and the repository index (next).

comment:5 in reply to:  4 Changed 8 years ago by Christian Boos

follow-up…

… In this use case, I believe it make sense to display both the default repository (first) and the repository index (next).

And yes, this use case could well need to be enabled explicitly, e.g. [browser] inline_default_repository = true, defaulting to false.

comment:6 Changed 8 years ago by Remy Blank

I like the idea of displaying the default repository in bold. I'm not sure about automatically creating an alias using the basename of the repository: I would rather define it explicitly if I want to. In the last (fourth) case you mention in comment:4, I wouldn't want any alias for the default repository at all, and this wouldn't be possible if the alias is created automatically.

I was actually going to go with the .browseable idea that you mentioned at the bottom of MultipleRepositorySupport, which would allow hiding any repository or alias, including the default one.

But that's actually orthogonal. How about the following:

  • Remove the (default) entry at the top of the list of repositories.
  • Mark aliases for (default) as bold.
  • Add a .browseable attribute to repositories and aliases that controls their visibility in the repository index.

comment:7 Changed 8 years ago by Christian Boos

Ok, I think we're getting there… Dropping the idea of an automatic alias for the default repository.

If there's no explicit alias defined for the default repository (case 4), then we show the default repository inlined.

If there's an alias defined, then we're not showing the default repository inline, so no need for a specific setting.

But then, we would need to have that alias listed in the repository index, and currently aliases are not listed. So yes, an extra browseable flag/metadata would be useful here. By default, 'browseable' should be true for repositories and false for aliases, except if this is an alias for the default repository.

comment:8 in reply to:  7 ; Changed 8 years ago by Remy Blank

Replying to cboos:

But then, we would need to have that alias listed in the repository index, and currently aliases are not listed.

Are you sure? I can see them here…

comment:9 in reply to:  8 ; Changed 8 years ago by Christian Boos

Replying to rblank:

Replying to cboos:

But then, we would need to have that alias listed in the repository index, and currently aliases are not listed.

Are you sure? I can see them here…

Actually, I had them defined in [repositories] and those seem to not work anymore, so maybe only the aliases defined in the repository table work for now.

comment:10 in reply to:  9 Changed 8 years ago by Remy Blank

Replying to cboos:

Actually, I had them defined in [repositories] and those seem to not work anymore, so maybe only the aliases defined in the repository table work for now.

Strange, I have aliases both in [repositories] and the repository table, and all are visible. Could it be that the repositories they point to don't exist anymore (or have been renamed)?

Changed 8 years ago by Remy Blank

Attachment: 8536-hide-repos-r8439.patch added

Add possibility to hide repositories from the index, and clean up index.

comment:11 Changed 8 years ago by Remy Blank

The patch above adds the following functionality:

  • Repositories and aliases can be hidden from the repository index, either with a .hidden attribute if they are defined in trac.ini, or with a checkbox in the admin panel.
  • Aliases for the default repository are shown in bold in the index.
  • If it is visible, the default repository is now shown at the top of the page instead of at the bottom.

Testing welcome!

comment:12 Changed 8 years ago by John Hampton

Applied the patch and I like it. I haven't given it a thorough testing as I don't have bunches of aliases, etc. But it took care of the default repos.

I did have to remove the repository from the ini and add it via the web interface to make it not the default repos. However, as long as we document this, I think it's an acceptable step.

+1 on the patch from me

comment:13 Changed 8 years ago by Christian Boos

The patch looks great. It's nearly OK.

First, I also had some trouble disabling the default repository. I initially tried to add an alias for it, but that's not enough to do the hide. Fine, but then we should have another easy way to hide it. I managed to do it this way:

[repositories]
.dir = <same dir as repository_dir>
.hidden = true

which amounts to the same solution as what John did, but through the .ini.

Minor trouble is that the code only test for the presence of the .hidden flag, so setting .hidden = false won't make the default repository visible again.

I also found out what happened to my aliases: I had them written alias = target, but now this has changed to name.alias = target. There's a minor glitch with that, though, as the aliased name is taken to be name.alias.

So I propose the additional changes:

  • trac/versioncontrol/api.py

     
    375375                dotindex = option.rindex('.')
    376376                name, detail = option[:dotindex], option[dotindex+1:]
    377377                if name in reponames:
    378                     reponames[name][detail] = repositories.get(option)
     378                    if 'detail' == 'hidden':
     379                        value = repositories.getbool(option)
     380                    else:
     381                        value = repositories.get(option)
     382                    reponames[name][detail] = value
    379383                elif detail == 'alias':
    380384                    alias = repositories.get(option)
    381385                    if alias in reponames:
    382                         reponames[option] = {'alias': alias}
     386                        reponames[name] = {'alias': alias}
    383387        # eventually add pre-0.12 default repository
    384388        if '' not in reponames and self.repository_dir:
    385389            reponames[''] = {'dir': self.repository_dir}

comment:14 in reply to:  13 ; Changed 8 years ago by Remy Blank

Replying to cboos:

I managed to do it this way:

Yes, I forgot to mention that. It's also the reason why I removed the [trac] repository_url config option. I assumed that the 0.12 release notes would mention that repositories should be defined in the [repositories] section, and that the [trac] repository_* options are deprecated.

We could also allow specifying the default repository as (default).dir, maybe this is slightly more intuitive.

Minor trouble is that the code only test for the presence of the .hidden flag, so setting .hidden = false won't make the default repository visible again.

Yes, I have already fixed that, but haven't updated the patch yet, sorry. (In your patch, 'detail' == 'hidden' will probably always be False ;-)

I also found out what happened to my aliases: I had them written alias = target, but now this has changed to name.alias = target. There's a minor glitch with that, though, as the aliased name is taken to be name.alias.

Ah, now I understand. I have always defined aliases as follows, which in retrospect is not very intuitive, but currently produces the correct results:

[repositories]
myalias.alias = myrepository
myalias.dir =

Whereas, if I understand you correctly, aliases were previously defined as:

[repositories]
myalias = myrepository

And your proposed change hits the middle ground, that is:

[repositories]
myalias.alias = myrepository

IMO, the second (initial) "syntax" was the most intuitive. Do you mind if I try to revert the code to that state? I assume it was me who inadvertently introduced the change in the first place.

comment:15 in reply to:  14 ; Changed 8 years ago by Christian Boos

Replying to rblank:

Replying to cboos:

I managed to do it this way:

Yes, I forgot to mention that. It's also the reason why I removed the [trac] repository_url config option. I assumed that the 0.12 release notes would mention that repositories should be defined in the [repositories] section, and that the [trac] repository_* options are deprecated.

We could do that yes, but then also mention it in the doc string for the option. We should also have a TracIni#repositories-section (0.12/TracIni for now).

We could also allow specifying the default repository as (default).dir, maybe this is slightly more intuitive.

Not sure about that. I think I prefer .dir = ....

Minor trouble is that the code only test for the presence of the .hidden flag, so setting .hidden = false won't make the default repository visible again.

Yes, I have already fixed that, but haven't updated the patch yet, sorry. (In your patch, 'detail' == 'hidden' will probably always be False ;-)

No idea why it worked yesterday, and of course no longer today… Maybe because now I know there's a bug :-) Magic…

I also found out what happened to my aliases: I had them written alias = target, but now this has changed to name.alias = target. There's a minor glitch with that, though, as the aliased name is taken to be name.alias.

Ah, now I understand. I have always defined aliases as follows, which in retrospect is not very intuitive, but currently produces the correct results: <snip> IMO, the second (initial) "syntax" was the most intuitive. Do you mind if I try to revert the code to that state?

No problem. The alias = target syntax also matches the [intertrac] one.

comment:16 in reply to:  15 Changed 8 years ago by Remy Blank

Replying to cboos:

We could do that yes, but then also mention it in the doc string for the option. We should also have a TracIni#repositories-section (0.12/TracIni for now).

Ok, I'll add to the docstring and documentation.

Not sure about that. I think I prefer .dir = ....

Ok.

No idea why it worked yesterday, and of course no longer today… Maybe because now I know there's a bug :-) Magic…

Heh, people often say that when my looking over their shoulder makes all problems go away ;-)

comment:17 Changed 8 years ago by Remy Blank

I have a new patch almost ready, but reading the docstring for [trac] repository_dir, I wonder if we shouldn't remove the special case in Environment.is_component_enabled() for enabling the trac.versioncontrol.* components based on repository_dir. I find it confusing to have to define an empty option to enable some components, and would prefer the trac.versioncontrol.* components would follow the normal rules of being enabled by default, and to be able to disable them with a line in [components].

Changed 8 years ago by Remy Blank

Attachment: 8536-hide-repos-r8441.patch added

Improved patch fixing aliases and .hidden values.

comment:18 Changed 8 years ago by Remy Blank

The patch above improves the following aspects:

  • .hidden values can now be any of the values defined in _TRUE_VALUES.
  • Repository aliases can again be defined with alias = repo.
  • The trac.versioncontrol.* components are now enabled by default like other trac.* components, and do not depend on [trac] repository_dir anymore.
  • [trac] repository_dir is deprecated in favor of [repositories].

comment:19 in reply to:  18 Changed 8 years ago by Christian Boos

Reviewed and tested the patch (comment:18), it's all good! Please apply.

Perhaps as a further documentation improvement, we could say a word in [trac] repository_type that it'll be also used as the default for entries defined in [repositories]. Also, have [repositories] in those docstrings link to the corresponding section in TracIni where we would describe the syntax, how to set up the a default repository, how to setup aliases, etc.

comment:20 in reply to:  18 ; Changed 8 years ago by John Hampton

Replying to rblank:

  • The trac.versioncontrol.* components are now enabled by default like other trac.* components, and do not depend on [trac] repository_dir anymore.

OK, I haven't looked at the code, but wasn't the point of disabling versioncontrol if repository was blank so that we could initenv without needing subversion installed?

I like the idea of the enabling/disabling be explicit, but we need to make sure we don't break the ability to install trac without a versioncontrol backend installed.

Again, haven't tested the patch, just wanted to make sure this was accounted for.

comment:21 in reply to:  20 ; Changed 8 years ago by Remy Blank

Replying to jhampton:

OK, I haven't looked at the code, but wasn't the point of disabling versioncontrol if repository was blank so that we could initenv without needing subversion installed?

Ah, good point. I'll check that.

OTOH, wouldn't it make sense to remove the questions about the repository from initenv altogether, now that the repositories can be defined in the admin panels or through trac-admin $ENV config set?

Also, should we disable the Browse Source menu when no repositories are defined at all?

comment:22 in reply to:  21 Changed 8 years ago by John Hampton

Replying to rblank:

OTOH, wouldn't it make sense to remove the questions about the repository from initenv altogether, now that the repositories can be defined in the admin panels or through trac-admin $ENV config set?

I think that's an acceptable solution.

Also, should we disable the Browse Source menu when no repositories are defined at all?

Yes, if there are no repositories, then there is no reason to have the Browse Source link.

comment:23 Changed 8 years ago by Remy Blank

Resolution: fixed
Status: newclosed

Improved patch applied in [8447], taking into account all the comments on this ticket. In particular, trac-admin $ENV initenv now doesn't ask for repository info anymore. I have checked the creation of environments without SVN bindings.

comment:24 Changed 8 years ago by Christian Boos

Milestone: 0.120.12-multirepos

comment:25 Changed 7 years ago by Jayen <jayen@…>

Hi, I recently upgraded to 0.12 and am trying to get a certain behaviour (related to this ticket's changes), but am having a little trouble following what has been done here and how I can configure trac to get what I want. I want the contents of the default alias, along with the repository index to show in the source browser. I've managed to get something close by duplicating a repository, without using an alias. I have:

.dir    = /ABC
abc.dir = /ABC
xyz.dir = /XYZ

What I would like to have is:

.alias     = abc
abc.dir    = /ABC
xyz.dir    = /XYZ
.hidden    = false
abc.hidden = false
xyz.hidden = false

(examples above are stored in the trac.db, and not in trac.ini.)

Is it possible to get the behaviour that this ticket complained about? (I am the crazy person who finds this useful.)

comment:26 in reply to:  25 Changed 7 years ago by Remy Blank

Replying to Jayen <jayen@…>:

Is it possible to get the behaviour that this ticket complained about?

It seems to be working here (on 0.12.2rc1), with the following section:

[repositories]
.alias = svn
svn.dir = /path/to/svn

Note that you can't define aliases in the [repositories] section to repositories defined in the "Repositories" admin panel.

comment:27 Changed 7 years ago by Jayen <jayen@…>

Interesting. Here is a dump from the db with a default [duplicate] repository, that shows, in the source browser, the default repository contents, above the repository index:

sqlite> select * from repository;
2|name|rescue
2|dir|/data/svn/rescue
2|type|svn
3|name|Mica
3|dir|/data/git/Mica
3|type|git
4|name|libcas
4|dir|/data/git/libcas
4|type|git
5|name|Experimental Robotics 2010
5|dir|/data/svn/comp4411/2010-s1
5|type|svn
6|name|
6|dir|/data/svn/rescue
6|type|svn

And here is a dump from the db with a default alias, that shows, in the source browser, the default repository contents, but no repository index:

sqlite> select * from repository;
2|name|rescue
2|dir|/data/svn/rescue
2|type|svn
3|name|Mica
3|dir|/data/git/Mica
3|type|git
4|name|libcas
4|dir|/data/git/libcas
4|type|git
5|name|Experimental Robotics 2010
5|dir|/data/svn/comp4411/2010-s1
5|type|svn
6|name|
6|dir|
6|alias|rescue

(dumps above strips youngest_rev and repository_dir entries for readability)

Is it possible it works with trac.ini, but not in trac.db? Should I try trac.ini and file a bug report, if I find that I can't get the desired behaviour configuring the repositories from the admin panel?

Thanks, Jayen

comment:28 in reply to:  27 Changed 7 years ago by Remy Blank

Replying to Jayen <jayen@…>:

6|name|
6|dir|
6|alias|rescue

Works fine here with the same entries, both the default repository and the repository index are shown.

If you still have entries in your [repositories] section, try removing them all and defining all repositories in the admin panel.

Do you have any authz checking? An alias is just a pointer, so if your default repository is an alias, you need to reference the aliased repository name in your authz file.

Which Trac version are you using? If you are still on 0.12, you may want to upgrade to 0.12.2rc1.

At this point, it starts looking like an InstallationIssue. I would suggest you raise the question on the trac-users MailingList or on the IrcChannel, so that you reach a wider audience.

comment:29 Changed 7 years ago by Jayen <jayen@…>

I don't have a [repositories] section (and I never did). I followed the instructions at http://trac.edgewall.org/wiki/TracRepositoryAdmin#Migration (using the admin panel) when I moved from 0.11.7 to 0.12.1. I'm not using authz.

I'll give 0.12.2rc1 a try and post my results to the mailing list if I still have trouble.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted.
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.