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. -- 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