Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Junio C Hamano wrote: > Felipe Contreras writes: > > > Junio C Hamano wrote: > >> Felipe Contreras writes: > >> > >> > *You* said this[1]: > >> > >> If you read the context you omitted from the quote, and realize that > >> it was a counter-suggestion to give a middle ground to a more > >> draconian "let's divide them into two", neither which I said I want > >> to see go forward immediately, you see that this message does not > >> deserve any response. > > > > So when you came with a guideline, that was only for git-remote-hg/bzr. > > There is no guideline involved. Exactly. It's all arbitrary. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Felipe Contreras writes: > Junio C Hamano wrote: >> Felipe Contreras writes: >> >> > *You* said this[1]: >> >> If you read the context you omitted from the quote, and realize that >> it was a counter-suggestion to give a middle ground to a more >> draconian "let's divide them into two", neither which I said I want >> to see go forward immediately, you see that this message does not >> deserve any response. > > So when you came with a guideline, that was only for git-remote-hg/bzr. There is no guideline involved. Go re-read the message upthread and find this part: In any case, that suggestion to remove not related to the "stick", either, and certeinly not about "prove yourself" purge that does not even exist. I have no more time nor desire to respond to you today. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Junio C Hamano wrote: > Felipe Contreras writes: > > > *You* said this[1]: > > If you read the context you omitted from the quote, and realize that > it was a counter-suggestion to give a middle ground to a more > draconian "let's divide them into two", neither which I said I want > to see go forward immediately, you see that this message does not > deserve any response. So when you came with a guideline, that was only for git-remote-hg/bzr. If you apply that guideline to other tools, you would have to remove them too, but that won't happen regarlesss of how crappy or well maintained they are out-of-tree already. Why? Because you say so. OK. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Felipe Contreras writes: > *You* said this[1]: If you read the context you omitted from the quote, and realize that it was a counter-suggestion to give a middle ground to a more draconian "let's divide them into two", neither which I said I want to see go forward immediately, you see that this message does not deserve any response. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Junio C Hamano wrote: > There is no "prove yourself is worthy or get evicted" purge going on > in the contrib/ area. I saw contrib/README referred to a few times > in the near-by threads, and I think these patches are done primarily > by deliberately misinterpreting one part of it in order to grab > attention by many people and also to sabotage the project. *You* said this[1]: - Eject tools in contrib/ that would benefit the users better if they were outside my tree. There are a few points to consider when judging "benefit better if outside": * Their release cycle requirements are better met outside my tree (the "remote-hg depends not just on Git but Hg internal" issue we have discussed). * They are actively maintained. The overall Git maintainer would merely be being a bottleneck than being a helpful editor with respect to these tools if we keep them in my tree, and we expect that the tool maintainer would do a much better job without me. - Keep tools that are not actively maintained but still used by the users widely in my tree, but when their external dependencies become baggage to Git as a whole, demote them to contrib/ and stop installing them by default. - I would not mind having install.contrib-frotz target in the top-level Makefile for each of the remaining contrib/frotz hierarchies for those users and distro packagers who know their platform meets the dependency requirements. So make up your mind. Which tools should be ejected from contrib and for what reasons? > The contrib/README file was written back when Git was still a small > and young project If contrib/README is not appropriate, then rewrite it. Having a maintainer making decisions about what goes in and goes outs arbitrarily helps no one. Or just remove it and be done with the pretense of haing any consistency. > The sole mention of possible removal from contrib/ is this one: Now you are contradicting what you said in [1]. Surely git-remote-hg/bzr aren't the only tools that meet the criteria you set in [1]. > in which Felipe said: > > I don't want to do anything for a "contrib" tool. > > and I suggested that he has an option to make it a standalone > third-party project. You are twisting the events incredibly. *You* started by threatening the removal[2]: > Having said that, I agree with the conclusion of your message:... > and I am inclined to be persuaded that the users of remote-hg/bzr > may better off if they are unbundled from my tree. I said I wasn't interested in working on this *after* you said they were not going to the core, and they should move out-of-tree. > that is one of the only two alternatives I can offer, given that the > Git ecosystem has matured enough to let third-party tools flourish > on their own merit. But it hasn't matured enough. That's *YOUR ASSUMPTION*. Look at all the fuzz my patch series has created. Does it seem to you these are the symptoms of an ecosystem mature enough to let third-party tools to flourish? If you think so, then let's continue cleaning up contrib. These tools will "flourish" according to you. > In any case, that suggestion to remove not related to the "stick", > either, and certeinly not about "prove yourself" purge that does not > even exist. > > So I think most of these removal patches can safely be ignored. Excellent, so you agree you engage in double standards. Tools stay in the core even when they haven't proven themselves (and even without tests), tools get dropped from the tree even when they have proven themselves. Got it. [1] http://article.gmane.org/gmane.comp.version-control.git/248233 [2] http://article.gmane.org/gmane.comp.version-control.git/248242 -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Jeff King writes: > On Thu, May 08, 2014 at 07:58:30PM -0500, Felipe Contreras wrote: > >> No activity since 2012, no tests, no chance of ever graduating. > > I don't think "no activity" is an interesting indicator. This tool _is_ > actively maintained, but it has not needed any fixes since 2012. I use > it for every single "git log" and "git diff" invocation I do via the > pager.* config. > > If we are getting rid of contrib/ I would be happy to continue > maintaining it out-of-tree. I do not know how much attention you have been paying, and I suspect that you may be aware of all of the following, but I'll send this out anyway, primarily so that others involved in other subthreads can find out the story behind this. There is no "prove yourself is worthy or get evicted" purge going on in the contrib/ area. I saw contrib/README referred to a few times in the near-by threads, and I think these patches are done primarily by deliberately misinterpreting one part of it in order to grab attention by many people and also to sabotage the project. The contrib/README file was written back when Git was still a small and young project that was trying to build an ecosystem by having an area to host stuff that are not core-material for some reason or other (e.g. only useful in some environments, only useful for some workflows, the design or code not up to par to be in core) in my tree to ease discovery and distribution. There, I wrote: I expect that things that start their life in the contrib/ area to graduate out of contrib/ once they mature, either by becoming projects on their own, or moving to the toplevel directory. On the other hand, I expect I'll be proposing removal of disused and inactive ones from time to time. The purpose the last sentence in that paragraph is there was to protect our codebase and our users from those who see an opportunity to throw their ware in to our tree and go AWOL, by giving me, the maintainer, a "stick" to prod them, saying "You as the primary author are responsible for taking good care of the ware you created by responding to issues (questions, suggestions, bugs, patches) in a prompt manner, or your ware may even get evicted." Among contrib/ materials we have today, I do not think there is anything that requires me to exercise that "stick". diff-highlight certainly is not. Perhaps subtree is the closest, as I see issues raised from time to time but the original champion seems to be inactive for some time, but even there, I recently saw somebody hinting to volunteer to take it over after sending a patch or two to it, and I do not intend to exercise the "stick" yet. The sole mention of possible removal from contrib/ is this one: http://thread.gmane.org/gmane.comp.version-control.git/248063/focus=248457 in which Felipe said: I don't want to do anything for a "contrib" tool. and I suggested that he has an option to make it a standalone third-party project. With the promotion to the core has already been ruled out in the thread that begins at this one: http://thread.gmane.org/gmane.comp.version-control.git/247660/focus=248167 that is one of the only two alternatives I can offer, given that the Git ecosystem has matured enough to let third-party tools flourish on their own merit. "We may want a better plug-in registry for Git" I mentioned in http://thread.gmane.org/gmane.comp.version-control.git/248063/focus=248391 was to help us in that direction, but seeing that imerge mentioned in many places I do not even regularly visit with the current "discovery and distribution" infrastructure, perhaps yet another new registry may not even be necessary. I dunno. In any case, that suggestion to remove not related to the "stick", either, and certeinly not about "prove yourself" purge that does not even exist. So I think most of these removal patches can safely be ignored. I agree with you and Jonathan that removal of contrib/vim may be a good idea, but that is not due to "stick" nor "prove yourself", either. Jonathan's proposed alternative $gmane/248506 does a good job of explaining and justifying the change. It is a graduation "by becoming projects on their own" that contrib/README mentions. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Stefan Beller wrote: > > > > It is serious. > > > > The purpose of the 'contrib/' area is not clear. The statemens coming > > from Junio don't match what is on 'contrib/README'. So we have a huge > > variance of quality all over 'contrib/'. Some tools in contrib have > > higher quality than what is part of the core (e.g. they have tests, > > while git-archimport doesn't). > > How about rewriting the README then? To say what? > Also as I said in another mail, we could split up the contrib/ area > into multiple areas with narrow defined use-cases, i.e. staging/, > Documentation/historicTools (the current contrib/examples section), > 3rdPartyTools/, Bridges/ (for cooperating with other VCS). That won't fix the issue that these tools are not maintained. A separate repository 'git-cruft' might do. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
> > It is serious. > > The purpose of the 'contrib/' area is not clear. The statemens coming > from Junio don't match what is on 'contrib/README'. So we have a huge > variance of quality all over 'contrib/'. Some tools in contrib have > higher quality than what is part of the core (e.g. they have tests, > while git-archimport doesn't). How about rewriting the README then? Also as I said in another mail, we could split up the contrib/ area into multiple areas with narrow defined use-cases, i.e. staging/, Documentation/historicTools (the current contrib/examples section), 3rdPartyTools/, Bridges/ (for cooperating with other VCS). > > So this is a serious attempt at making sure we remain consistent through > the core and contrib/. > > Personally I would like to see everything in contrib/ have *at least* > some tests and documentation. Otherwise we know what's going to happen; > they are going to rot and nobody will care that the tools don't work any > more, or they won't even know what they do and how to use them. > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
Jeff King wrote: > On Thu, May 08, 2014 at 07:58:30PM -0500, Felipe Contreras wrote: > > > No activity since 2012, no tests, no chance of ever graduating. > > I don't think "no activity" is an interesting indicator. This tool _is_ > actively maintained, but it has not needed any fixes since 2012. I use > it for every single "git log" and "git diff" invocation I do via the > pager.* config. If this tool is important to you, I'd say you should write tests so that we (the rest of the world) can verify that it still works properly. If it does, it might have a chance of graduating to the core, so it should stay. > If we are getting rid of contrib/ I would be happy to continue > maintaining it out-of-tree. But I honestly cannot tell if this thread is > serious or passive-aggressive posturing. It is serious. The purpose of the 'contrib/' area is not clear. The statemens coming from Junio don't match what is on 'contrib/README'. So we have a huge variance of quality all over 'contrib/'. Some tools in contrib have higher quality than what is part of the core (e.g. they have tests, while git-archimport doesn't). So this is a serious attempt at making sure we remain consistent through the core and contrib/. Personally I would like to see everything in contrib/ have *at least* some tests and documentation. Otherwise we know what's going to happen; they are going to rot and nobody will care that the tools don't work any more, or they won't even know what they do and how to use them. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 19/25] contrib: remove 'diff-highlight'
On Thu, May 08, 2014 at 07:58:30PM -0500, Felipe Contreras wrote: > No activity since 2012, no tests, no chance of ever graduating. I don't think "no activity" is an interesting indicator. This tool _is_ actively maintained, but it has not needed any fixes since 2012. I use it for every single "git log" and "git diff" invocation I do via the pager.* config. If we are getting rid of contrib/ I would be happy to continue maintaining it out-of-tree. But I honestly cannot tell if this thread is serious or passive-aggressive posturing. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 19/25] contrib: remove 'diff-highlight'
No activity since 2012, no tests, no chance of ever graduating. Cc: Jeff King Signed-off-by: Felipe Contreras --- contrib/diff-highlight/README | 152 - contrib/diff-highlight/diff-highlight | 173 -- 2 files changed, 325 deletions(-) delete mode 100644 contrib/diff-highlight/README delete mode 100755 contrib/diff-highlight/diff-highlight diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README deleted file mode 100644 index 502e03b..000 --- a/contrib/diff-highlight/README +++ /dev/null @@ -1,152 +0,0 @@ -diff-highlight -== - -Line oriented diffs are great for reviewing code, because for most -hunks, you want to see the old and the new segments of code next to each -other. Sometimes, though, when an old line and a new line are very -similar, it's hard to immediately see the difference. - -You can use "--color-words" to highlight only the changed portions of -lines. However, this can often be hard to read for code, as it loses -the line structure, and you end up with oddly formatted bits. - -Instead, this script post-processes the line-oriented diff, finds pairs -of lines, and highlights the differing segments. It's currently very -simple and stupid about doing these tasks. In particular: - - 1. It will only highlight hunks in which the number of removed and - added lines is the same, and it will pair lines within the hunk by - position (so the first removed line is compared to the first added - line, and so forth). This is simple and tends to work well in - practice. More complex changes don't highlight well, so we tend to - exclude them due to the "same number of removed and added lines" - restriction. Or even if we do try to highlight them, they end up - not highlighting because of our "don't highlight if the whole line - would be highlighted" rule. - - 2. It will find the common prefix and suffix of two lines, and - consider everything in the middle to be "different". It could - instead do a real diff of the characters between the two lines and - find common subsequences. However, the point of the highlight is to - call attention to a certain area. Even if some small subset of the - highlighted area actually didn't change, that's OK. In practice it - ends up being more readable to just have a single blob on the line - showing the interesting bit. - -The goal of the script is therefore not to be exact about highlighting -changes, but to call attention to areas of interest without being -visually distracting. Non-diff lines and existing diff coloration is -preserved; the intent is that the output should look exactly the same as -the input, except for the occasional highlight. - -Use - -You can try out the diff-highlight program with: - -- -git log -p --color | /path/to/diff-highlight -- - -If you want to use it all the time, drop it in your $PATH and put the -following in your git configuration: - -- -[pager] - log = diff-highlight | less - show = diff-highlight | less - diff = diff-highlight | less -- - -Bugs - - -Because diff-highlight relies on heuristics to guess which parts of -changes are important, there are some cases where the highlighting is -more distracting than useful. Fortunately, these cases are rare in -practice, and when they do occur, the worst case is simply a little -extra highlighting. This section documents some cases known to be -sub-optimal, in case somebody feels like working on improving the -heuristics. - -1. Two changes on the same line get highlighted in a blob. For example, - highlighting: - --- --foo(buf, size); -+foo(obj->buf, obj->size); --- - - yields (where the inside of "+{}" would be highlighted): - --- --foo(buf, size); -+foo(+{obj->buf, obj->}size); --- - - whereas a more semantically meaningful output would be: - --- --foo(buf, size); -+foo(+{obj->}buf, +{obj->}size); --- - - Note that doing this right would probably involve a set of - content-specific boundary patterns, similar to word-diff. Otherwise - you get junk like: - -- --this line has some -{i}nt-{ere}sti-{ng} text on it -+this line has some +{fa}nt+{a}sti+{c} text on it -- - - which is less readable than the current output. - -2. The multi-line matching assumes that lines in the pre- and post-image - match by position. This is often the case, but can be fooled