On Wed, Dec 18, 2013 at 7:18 PM, Tom Miller <jackerran@xxxxxxxxx> wrote: > On Wed, Dec 18, 2013 at 3:47 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Tom Miller <jackerran@xxxxxxxxx> writes: >> >>> In order to fix branchname DF conflicts during `fetch --prune`, the way >>> the header is output to the screen needs to be refactored. Here is an >>> exmaple of the output with the line in question denoted by '>': >>> >>> $ git fetch --prune --dry-run upstream >>>> From https://github.com/git/git >>> a155a5f..5512ac5 maint -> upstream/maint >>> d7aced9..7794a68 master -> upstream/master >>> 523f7c4..3e57c29 next -> upstream/next >>> + 462f102...0937cdf pu -> upstream/pu (forced update) >>> e24105a..5d352bc todo -> upstream/todo >>> * [new tag] v1.8.5.2 -> v1.8.5.2 >>> * [new tag] v1.8.5.2 -> v1.8.5.2 >>> >>> pretty_url(): >>> This function when passed a transport url will anonymize the transport >>> of the url. It will strip a trailing '/'. It will also strip a trailing >>> '.git'. It will return the newly formated url for use. I do not believe >>> there is a need for stripping the trailing '/' and '.git' from a url, >>> but it was already there and I wanted to make as little changes as >>> possible. >> >> OK. I tend to agree that stripping the trailing part is probably >> not a good idea and we would want to remove that but that definitely >> should be done as a separate step, or even as a separate series on >> top of this one. > > I think that removing the trailing part will greatly reduce the complexity > to the point were it is unnecessary to have pretty_url(). My goal with > extracting this function is to isolate the complexity of formatting the > url to a single spot. I am thinking along the lines of the following > commit order: > > 1. Remove the "remove trailing part" > 2. Add print_url() > 3. Always print url when pruning > 4. Reverse order of prune and fetch > >>> print_url(): >>> This function will convert a transport url to a pretty url using >>> pretty_url(). Then it will print out the pretty url to stderr as >>> indicated above in the example output. It uses a global variable >>> named "gshown_url' to prevent this header for being printed twice. >> >> Gaah. What is that 'g' doing there? Please don't do that >> meaningless naming. > > I am not familiar with C conventions and I was trying to stay consistent. > I saw other global variables starting with 'g' and made an assumption. > It will use the original name in the upcoming patches. > >> I do not think the change to introduce such a global variable >> belongs to this refactoring step. The current caller can decide >> itself if it called that function, and if you are going to introduce >> new callers in later steps, they can coordinate among themselves, >> no? > > I agree, there is no reason for introducing it in this step. Thanks for > pointing that out. After working on this some more and realizing there is more work to be done on the "fetch --prune should prune before fetching" issue. Also, seeing Jeff's response opened my eyes even more. I believe you are correct. The "trailing parts" piece should be split off into another patch set. I think it would make sense to add the "fetch --prune should print the header url" to that patch set. Should I submit those patches as a separate thread or reply to this thread with just those patches? -- Tom Miller jackerran@xxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html