On Mon, Jan 21, 2013 at 1:58 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote: > On Sat, Jan 19, 2013 at 09:04:57AM +0000, Blue Swirl wrote: >> On Fri, Jan 18, 2013 at 11:04 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote: >> > On Thu, Jan 17, 2013 at 08:13:38PM +0000, Blue Swirl wrote: >> >> On Mon, Jan 14, 2013 at 9:09 AM, Stefan Hajnoczi <stefa...@gmail.com> >> >> wrote: >> >> > On Sat, Jan 12, 2013 at 10:46:18AM +0000, Blue Swirl wrote: >> >> >> On Thu, Jan 10, 2013 at 12:43 PM, Stefan Hajnoczi <stefa...@gmail.com> >> >> >> wrote: >> >> >> > On Wed, Jan 09, 2013 at 08:43:51PM +0000, Blue Swirl wrote: >> >> >> >> On Tue, Jan 8, 2013 at 9:03 AM, Stefan Hajnoczi >> >> >> >> <stefa...@gmail.com> wrote: >> >> >> >> > On Mon, Jan 07, 2013 at 03:38:39PM -0500, Nickolai Zeldovich >> >> >> >> > wrote: >> >> >> >> >> memcpy() for overlapping regions is undefined behavior; use >> >> >> >> >> memmove() >> >> >> >> >> instead in readline_hist_add(). >> >> >> >> >> >> >> >> >> >> Signed-off-by: Nickolai Zeldovich <nicko...@csail.mit.edu> >> >> >> >> >> --- >> >> >> >> >> readline.c | 4 ++-- >> >> >> >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> > >> >> >> >> > I made a slight modification: keep the tab characters since the >> >> >> >> > surrounding code still uses them. >> >> >> >> >> >> >> >> I think tabs should be fixed whenever possible, otherwise we may >> >> >> >> never >> >> >> >> get them converted. >> >> >> > >> >> >> > Not in a one-line patch when the surrounding lines still use them. >> >> >> > It >> >> >> > creates a mess. >> >> >> >> >> >> Only if the reader messes with the tab width settings (and in that >> >> >> case they deserve what they get and they are probably also used to >> >> >> this), otherwise a line with tabs converted to spaces looks exactly >> >> >> the same. >> >> > >> >> > You are oversimplifying how tab widths work. The author and reader's >> >> > tab width must match in order for displayed text to appear correctly. >> >> >> >> Exactly. The default tab width is 8 in all tools and it takes some >> >> effort to adjust the tab settings in each tool. For example, how do >> >> you change it in less, xterm or cmd.exe? >> >> >> >> It is unreasonable and arrogant for an author to assume any other >> >> setting used by the reader for tab width, even if this was declared >> >> for example at the start of the file. >> >> >> >> Perhaps an analogy could be a compressing or encrypting preprocessor >> >> for the white space in the code. Without the right tool and correct >> >> settings, the reader would not see the white space in the code >> >> correctly. >> >> >> >> > Tell me what you consider the "correct" tab width for readers and I'll >> >> > find a piece of QEMU code that was authored for a different tab width >> >> > :). >> >> >> >> 8. >> >> >> >> > >> >> > In other words, it's a mess and best not to perturb it further. >> >> >> >> No, those messes should be cleaned up, much like braces. >> > >> > Agreed but in cleanup patches or when touching the whole function. Not >> > one line at a time because then we have an even bigger mess. >> >> So you are advocating cleanup patches which only adjust formatting? > > Yes. Also, cleaning up the entire function when making significant > changes is good.
Then we should require cleanup patches prior to patches which touch lines with tabs. Otherwise tabs will not be removed. >> How about mass conversion then? Anthony has rejected the similar >> approach for braces because of the loss of git blame info. > > There are drawbacks to mass converting code that isn't being otherwise > touched. It makes git-blame(1) harder to use and the patches still > require code review from the community. > > Best to convert files that are under active development, where it's > worth reformatting and we rebuild commit history quickly. > > For files that are rarely/never touched, the drawback can be bigger than > the advantage. > > Stefan