hello, there are many Python codes in tools/ that mix tab and space for indentation, which might introduce some silly and hidden bugs. this is a serious problem and must be avoid at all cost. here is a patch (against cset 1.1452) to replace all tabs with spaces. all the changes are pretty trivial, and i double checked everything. please take a look to see if all the modifies are OK. Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com> any idea? if nobody complains, i would prepare a patch like this for -testing tree. $ diffstat removetab.patch logging/logging-0.4.9.2/test/mymodule.py | 6 +- xen/sv/CreateDomain.py | 34 ++++++++-------- xen/sv/Daemon.py | 2 xen/sv/DomInfo.py | 55 ++++++++++++-------------- xen/sv/DomList.py | 16 +++---- xen/sv/GenTabbed.py | 6 +- xen/sv/HTMLBase.py | 4 - xen/sv/Main.py | 12 ++--- xen/sv/NodeInfo.py | 16 +++---- xen/sv/Wizard.py | 64 +++++++++++++++---------------- xen/sv/util.py | 6 +- xen/web/SrvBase.py | 6 +- xen/web/SrvDir.py | 2 xen/xend/EventServer.py | 24 +++++------ xen/xend/PrettyPrint.py | 6 +- xen/xend/XendClient.py | 6 +- xen/xend/XendDomain.py | 4 - xen/xend/encode.py | 2 xen/xend/server/SrvDomain.py | 6 +- xen/xend/sxp.py | 2 xen/xm/main.py | 12 ++--- 21 files changed, 143 insertions(+), 148 deletions(-) -- regards, aq _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jacob Gorm Hansen
2005-May-18 19:59 UTC
Re: [Xen-devel] [PATCH] replace tabs with spaces of Python code
aq wrote:> hello, > > there are many Python codes in tools/ that mix tab and space for > indentation, which might introduce some silly and hidden bugs. this is > a serious problem and must be avoid at all cost.I would suggest doing the opposite, like Linux does, but I guess this is mostly a matter of taste (i.e. your''s is bad and mine is good ;-)) Jacob _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Pratt
2005-May-18 20:06 UTC
RE: [Xen-devel] [PATCH] replace tabs with spaces of Python code
> there are many Python codes in tools/ that mix tab and space > for indentation, which might introduce some silly and hidden > bugs. this is a serious problem and must be avoid at all cost. > > here is a patch (against cset 1.1452) to replace all tabs with spaces. > all the changes are pretty trivial, and i double checked everything. > please take a look to see if all the modifies are OK. > > Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com>Are you sure python interprets a tab as a 4 spaces? I thought it interpretted it as 8? Hmm, there are a whole bunch of outstanding xend patches this this is going to create merge hell for, so I''m not sure we want to apply this straight away. Could you just post the script you used, or did you just do the obvious sed invocation? Thanks, Ian> any idea? if nobody complains, i would prepare a patch like > this for -testing tree. > > > $ diffstat removetab.patch > logging/logging-0.4.9.2/test/mymodule.py | 6 +- > xen/sv/CreateDomain.py | 34 ++++++++-------- > xen/sv/Daemon.py | 2 > xen/sv/DomInfo.py | 55 > ++++++++++++-------------- > xen/sv/DomList.py | 16 +++---- > xen/sv/GenTabbed.py | 6 +- > xen/sv/HTMLBase.py | 4 - > xen/sv/Main.py | 12 ++--- > xen/sv/NodeInfo.py | 16 +++---- > xen/sv/Wizard.py | 64 > +++++++++++++++---------------- > xen/sv/util.py | 6 +- > xen/web/SrvBase.py | 6 +- > xen/web/SrvDir.py | 2 > xen/xend/EventServer.py | 24 +++++------ > xen/xend/PrettyPrint.py | 6 +- > xen/xend/XendClient.py | 6 +- > xen/xend/XendDomain.py | 4 - > xen/xend/encode.py | 2 > xen/xend/server/SrvDomain.py | 6 +- > xen/xend/sxp.py | 2 > xen/xm/main.py | 12 ++--- > 21 files changed, 143 insertions(+), 148 deletions(-) > > -- > regards, > aq >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Vincent Hanquez
2005-May-18 20:21 UTC
Re: [Xen-devel] [PATCH] replace tabs with spaces of Python code
On Wed, May 18, 2005 at 09:06:37PM +0100, Ian Pratt wrote:> Hmm, there are a whole bunch of outstanding xend patches this this is > going to create merge hell for, so I''m not sure we want to apply this > straight away.The patch looks OK anyway. -- Vincent Hanquez _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2005-May-18 20:43 UTC
Re: [Xen-devel] [PATCH] replace tabs with spaces of Python code
Ian Pratt wrote:> > > > >>there are many Python codes in tools/ that mix tab and space >>for indentation, which might introduce some silly and hidden >>bugs. this is a serious problem and must be avoid at all cost. >> >>here is a patch (against cset 1.1452) to replace all tabs with spaces. >>all the changes are pretty trivial, and i double checked everything. >>please take a look to see if all the modifies are OK. >> >>Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com> >> >> > >Are you sure python interprets a tab as a 4 spaces? I thought it >interpretted it as 8? > >It''s not that simple. Check out 2.1.8 of the python language spec. http://docs.python.org/ref/indentation.html Tabs are expanded to the next 8 character column. The following script expands tabs as described in the python standard. I don''t know how to do tab expansion in sed. Regards, Anthony Liguori Signed-off-by: Anthony Liguori>Hmm, there are a whole bunch of outstanding xend patches this this is >going to create merge hell for, so I''m not sure we want to apply this >straight away. > >Could you just post the script you used, or did you just do the obvious >sed invocation? > >Thanks, >Ian > > > >>any idea? if nobody complains, i would prepare a patch like >>this for -testing tree. >> >> >>$ diffstat removetab.patch >> logging/logging-0.4.9.2/test/mymodule.py | 6 +- >> xen/sv/CreateDomain.py | 34 ++++++++-------- >> xen/sv/Daemon.py | 2 >> xen/sv/DomInfo.py | 55 >>++++++++++++-------------- >> xen/sv/DomList.py | 16 +++---- >> xen/sv/GenTabbed.py | 6 +- >> xen/sv/HTMLBase.py | 4 - >> xen/sv/Main.py | 12 ++--- >> xen/sv/NodeInfo.py | 16 +++---- >> xen/sv/Wizard.py | 64 >>+++++++++++++++---------------- >> xen/sv/util.py | 6 +- >> xen/web/SrvBase.py | 6 +- >> xen/web/SrvDir.py | 2 >> xen/xend/EventServer.py | 24 +++++------ >> xen/xend/PrettyPrint.py | 6 +- >> xen/xend/XendClient.py | 6 +- >> xen/xend/XendDomain.py | 4 - >> xen/xend/encode.py | 2 >> xen/xend/server/SrvDomain.py | 6 +- >> xen/xend/sxp.py | 2 >> xen/xm/main.py | 12 ++--- >> 21 files changed, 143 insertions(+), 148 deletions(-) >> >>-- >>regards, >>aq >> >> >> > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5/19/05, Jacob Gorm Hansen <jacobg@diku.dk> wrote:> aq wrote: > > hello, > > > > there are many Python codes in tools/ that mix tab and space for > > indentation, which might introduce some silly and hidden bugs. this is > > a serious problem and must be avoid at all cost. > > I would suggest doing the opposite, like Linux does, but I guess this is > mostly a matter of taste (i.e. your''s is bad and mine is good ;-))because most of code in Xen use spaces to indent, so it takes less time/effort to convert/check if we change tab to spaces. so it is wiser not to do the opposite :-) regards, aq _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5/19/05, Ian Pratt <m+Ian.Pratt@cl.cam.ac.uk> wrote:> > > > there are many Python codes in tools/ that mix tab and space > > for indentation, which might introduce some silly and hidden > > bugs. this is a serious problem and must be avoid at all cost. > > > > here is a patch (against cset 1.1452) to replace all tabs with spaces. > > all the changes are pretty trivial, and i double checked everything. > > please take a look to see if all the modifies are OK. > > > > Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com> > > Are you sure python interprets a tab as a 4 spaces? I thought it > interpretted it as 8?i investigated all the related code (not quite many), and they all use 4 spaces to indent. and in the above message, i didnt say that i convert tab to 4 spaces, either. see below why.> > Hmm, there are a whole bunch of outstanding xend patches this this is > going to create merge hell for, so I''m not sure we want to apply this > straight away. >not many Python files mixed with tabs, and we can check to fix them all. this should be cosidered bug, so no matter how hard it is, we must fix it finally. we cannot avoid this. mixing tabs and spaces is a *very* serious problem, if you care. when i reviewed code in my editor (vim in this case), almost all lines which have tabs dont look right to me. some even mislead me, and only when i turn on the hidden chars (":set list" in vim), i realized that there were tabs inside. ouch!> Could you just post the script you used, or did you just do the obvious > sed invocation? >actually, i did this job manually, with little help from one small script. the reasons are: - as you said, it is very dangeurous to do this kind of thing automatically, so it is best to do it by hand - the number of related files are not that many (21 files with nearly 150 related lines of code) - some files use tab as 8 spaces, some use tab as 4 spaces, some even mixed 4 and 8 spaces (checking by seeing the code structure). so it is another reason not to do it automatically by a script. Ian, it is a mistake if you dont get this patch. besides, the patch is rather small, and you can check to see if it is OK. below is a trivial script i used to indentify all tabs, so i dont miss any thing. i post here, incase it is helpful for somebody. regards, aq #### #! /usr/bin/env python import sys, os def main(): for filename in sys.argv[1:]: if os.path.isdir(filename): continue data = open(filename, "rb").read() if ''\0'' in data: continue if data.find("\t") != -1: lines = data.split(''\n'') print filename n = 0 for l in lines: n += 1 if l.find(''\t'') != -1: print ''\t'', n, ''\t'', l if __name__ == ''__main__'': main() _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Pratt
2005-May-19 01:28 UTC
RE: [Xen-devel] [PATCH] replace tabs with spaces of Python code
> actually, i did this job manually, with little help from one > small script. the reasons are: > - as you said, it is very dangeurous to do this kind of thing > automatically, so it is best to do it by handNo, I think I''d rather it was done automatically, by a script that interprets white space the same way that python does. The script from python.org that Anthony posted seems to fit the bill. It would be very interesting to compare the output of the script against your patch, to find out whether you fixed any bugs (or introduced any :-)> Ian, it is a mistake if you dont get this patch. besides, the > patch is rather small, and you can check to see if it is OK.I agree that getting rid of the tabs is important, but it can happen after some of the outstanding tools merges get completed. Of course, if in doing the conversion you find bugs, that''s a different matter... Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5/19/05, Ian Pratt <m+Ian.Pratt@cl.cam.ac.uk> wrote:> > actually, i did this job manually, with little help from one > > small script. the reasons are: > > - as you said, it is very dangeurous to do this kind of thing > > automatically, so it is best to do it by hand > > No, I think I''d rather it was done automatically, by a script that > interprets white space the same way that python does.of course it is fine if we have that kind of tool. but when we have no such one, we can fix them all by hand, and from then on (in the future) double check all the new python code before pushing them into repo, so we can make sure no such a problem ever occur again. as i said, since the patch is pretty small, we can afford to check it by hand.> The script from python.org that Anthony posted seems to fit the bill.no, that script doesnt work very well, because: 1. almost all the code indented by 4 spaces, but his script convert tab to 8 spaces 2. some code consider tab as 4 and 8 spaces in the same file, so it is hard to use this script. 3. that script doesnt see how python deal with the code. for example, lines the python code is aligned with the previous line, but the script doesnt take that into account. that is a problem. a small example: file tools/python/xen/xm/main.py has few tabs inside, and here is how Anthony''s script converts the code, and it does it wrongly. i have 3 files to compare here, so you can make a conclusion: the main.org.py (copy from original main.py), main-t2s.py (converted with Anthony''s script, with tabsize=4) and main-aq.py (my script, coverted manually) now to see the problem, here is a hunk taken from the diffs. see the commented i put next to the broken line: $ diff -Nurp main.org.py main-t2s.py --- main.org.py 2005-05-19 10:59:54.000000000 +0900 +++ main-t2s.py 2005-05-19 10:58:42.000000000 +0900 @@ -142,8 +142,8 @@ class Xm: """ self.name = args[0] if len(args) < 2: - args.append(''help'') - help = self.helparg(args) + args.append(''help'') + help = self.helparg(args) <==== wrongly indented p = self.getprog(args[1], self.unknown) if help or len(args) < 2: p.help(args[1:]) in the above code, the first tab is considered 4 spaces, but the second is considered 8 spaces. then problem arises with the Anthony''s script. $ diff -Nurp main.org.py main-aq.py --- main.org.py 2005-05-19 10:59:54.000000000 +0900 +++ main-aq.py 2005-05-19 11:01:20.000000000 +0900 @@ -142,8 +142,8 @@ class Xm: """ self.name = args[0] if len(args) < 2: - args.append(''help'') - help = self.helparg(args) + args.append(''help'') + help = self.helparg(args) p = self.getprog(args[1], self.unknown) if help or len(args) < 2: p.help(args[1:]) everything is fine with main-aq.py and here is a hunk got from diff between main-t2s.py and main-aq.py. cleary they are not same. $ diff -Nurp main-aq.py main-t2s.py --- main-aq.py 2005-05-19 11:01:20.000000000 +0900 +++ main-t2s.py 2005-05-19 10:58:42.000000000 +0900 @@ -143,7 +143,7 @@ class Xm: self.name = args[0] if len(args) < 2: args.append(''help'') - help = self.helparg(args) + help = self.helparg(args) p = self.getprog(args[1], self.unknown) if help or len(args) < 2: p.help(args[1:])> I agree that getting rid of the tabs is important, but it can happen > after some of the outstanding tools merges get completed.i think it is better to do it now, when there are not much code having problem. in the future, code get bigger, so it is more difficult to convert and check everything. regards, aq _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2005-May-19 09:17 UTC
Re: [Xen-devel] [PATCH] replace tabs with spaces of Python code
On 5/19/05, aq <aquynh@gmail.com> wrote:> > The script from python.org that Anthony posted seems to fit the bill. > > no, that script doesnt work very well, because: > 1. almost all the code indented by 4 spaces, but his script convert > tab to 8 spaces > 2. some code consider tab as 4 and 8 spaces in the same file, so it is > hard to use this script. > 3. that script doesnt see how python deal with the code. for example, > lines the python code is aligned with the previous line, but the > script doesnt take that into account. that is a problem. > > a small example: file tools/python/xen/xm/main.py has few tabs inside, > and here is how Anthony''s script converts the code, and it does it > wrongly. i have 3 files to compare here, so you can make a conclusion: > the main.org.py (copy from original main.py), main-t2s.py (converted > with Anthony''s script, with tabsize=4) and main-aq.py (my script, > coverted manually) > > now to see the problem, here is a hunk taken from the diffs. see the > commented i put next to the broken line:That''s because you broke the script by changing tabsize to 4. As Anthony wrote, the python manual describes the algorithm used to expand tabs and how tabs get expanded has nothing to with how you have indented your program. Whether your indention levels are 4 character wide or 8 doesn''t matter, a tab always means go to the next 8 character column. The original main.py looks visually incorrect at the args.append line (it''s indented by 8 characters instead of 4) but that''s not a bug.> > I agree that getting rid of the tabs is important, but it can happen > > after some of the outstanding tools merges get completed. > > i think it is better to do it now, when there are not much code having > problem. in the future, code get bigger, so it is more difficult to > convert and check everything.I''m not too keen on seeing this done in any of the 2.0 trees. christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5/19/05, Christian Limpach <christian.limpach@gmail.com> wrote:> On 5/19/05, aq <aquynh@gmail.com> wrote: > > > The script from python.org that Anthony posted seems to fit the bill. > > > > no, that script doesnt work very well, because: > > 1. almost all the code indented by 4 spaces, but his script convert > > tab to 8 spaces > > 2. some code consider tab as 4 and 8 spaces in the same file, so it is > > hard to use this script. > > 3. that script doesnt see how python deal with the code. for example, > > lines the python code is aligned with the previous line, but the > > script doesnt take that into account. that is a problem. > > > > a small example: file tools/python/xen/xm/main.py has few tabs inside, > > and here is how Anthony''s script converts the code, and it does it > > wrongly. i have 3 files to compare here, so you can make a conclusion: > > the main.org.py (copy from original main.py), main-t2s.py (converted > > with Anthony''s script, with tabsize=4) and main-aq.py (my script, > > coverted manually) > > > > now to see the problem, here is a hunk taken from the diffs. see the > > commented i put next to the broken line: > > That''s because you broke the script by changing tabsize to 4. As > Anthony wrote, the python manual describes the algorithm used to > expand tabs and how tabs get expanded has nothing to with how you have > indented your program. Whether your indention levels are 4 character > wide or 8 doesn''t matter, a tab always means go to the next 8 > character column. The original main.py looks visually incorrect at > the args.append line (it''s indented by 8 characters instead of 4) but > that''s not a bug.Yes, I agree. Actually Anthony''s script (with tabsize=8) can be used to convert all related Python files, and the result code would work like before.But since all the code originally indented with 4 spaces, then we should still readjust some code a little bit. If you want to play safely, I would rather use that script to remove tabs completely, than leave them as they are now (not buggy but *evil*). regards, aq _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
David Hopwood
2005-May-19 17:53 UTC
Re: [Xen-devel] [PATCH] replace tabs with spaces of Python code
Ian Pratt wrote:>>actually, i did this job manually, with little help from one >>small script. the reasons are: >>- as you said, it is very dangeurous to do this kind of thing >>automatically, so it is best to do it by hand > > No, I think I''d rather it was done automatically, by a script that > interprets white space the same way that python does.That assumes that the use of tabs is currently correct. -- David Hopwood <david.nospam.hopwood@blueyonder.co.uk> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel