Derby
  1. Derby
  2. DERBY-5896

Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.10.1.1
    • Fix Version/s: 10.10.1.1
    • Component/s: Network Server
    • Labels:
      None
    • Issue & fix info:
      Patch Available

      Description

      Change all the network server code under java/drda to use spaces instead of tabs.

      Having mixed tabs and spaces can be confusing, irritating and time consuming for developers especially in the network code where client is all spaces and the server mostly tabs.

      With the -x -b options on svn merge and svn diff, merges should be reasonable even if we change all the tabs to 4 spaces in the files under java/drda with a script something like (not yet tried):

      #!/bin/bash
      files=$@
      for file in $files

      { echo $file mv $file $file.orig sed -e 's/\t/ /g' < $file.orig > $file rm $file.orig }

      Are there any objections to this change? If not does anyone have any pending DRDA changes they would like to get in before I make the change?

      1. derby-5896-client-align-with-tab-stops.diff.txt
        7 kB
        Knut Anders Hatlen
      2. derby-5896-align-with-tab-stops.diff.txt
        191 kB
        Knut Anders Hatlen
      3. derby-5896_stat.txt
        2 kB
        Kathey Marsden
      4. derby-5896_diff.txt
        1.38 MB
        Kathey Marsden
      5. derby-5896_client_stat.txt
        2 kB
        Kathey Marsden
      6. derby-5896_client_diff.txt
        120 kB
        Kathey Marsden

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        [bulk update: close all resolved issues that haven't had any activity the last year]

        Show
        Knut Anders Hatlen added a comment - [bulk update: close all resolved issues that haven't had any activity the last year]
        Hide
        Kathey Marsden added a comment -

        Resolving for trunk with the following revisions:
        Thank you Knut for fixing the alignment!

        http://svn.apache.org/viewvc?view=revision&amp;revision=1370674
        ------------------------------------------------------------------------
        r1370674 | kmarsden | 2012-08-07 23:54:46 -0700 (Tue, 07 Aug 2012) | 3 lines

        DERBY-5896 Change java/drda source code to use spaces instead of tabs

        ------------------------------------------------------------------------
        http://svn.apache.org/viewvc?view=revision&amp;revision=1371306
        ------------------------------------------------------------------------
        r1371306 | kmarsden | 2012-08-09 09:41:32 -0700 (Thu, 09 Aug 2012) | 5 lines

        DERBY-5896 Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client

        Change client tabs to spaces.

        ------------------------------------------------------------------------
        http://svn.apache.org/viewvc?view=revision&amp;revision=1372814
        ------------------------------------------------------------------------
        r1372814 | kahatlen | 2012-08-14 02:53:09 -0700 (Tue, 14 Aug 2012) | 4 lines

        DERBY-5896: Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client

        Align with tab stops if the replaced tab character itself wasn't located on a tab stop.

        ------------------------------------------------------------------------
        http://svn.apache.org/viewvc?view=revision&amp;revision=1372841
        ------------------------------------------------------------------------
        r1372841 | kahatlen | 2012-08-14 04:17:14 -0700 (Tue, 14 Aug 2012) | 4 lines

        DERBY-5896: Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client

        Align with tab stops in java/client.

        ------------------------------------------------------------------------
        http://svn.apache.org/viewvc?view=revision&amp;revision=1372818
        ------------------------------------------------------------------------
        r1372818 | kahatlen | 2012-08-14 03:05:00 -0700 (Tue, 14 Aug 2012) | 4 lines

        DERBY-5896: Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client

        Align with tab stops if the replaced tab character itself wasn't located on a tab stop.

        ------------------------------------------------------------------------

        Show
        Kathey Marsden added a comment - Resolving for trunk with the following revisions: Thank you Knut for fixing the alignment! http://svn.apache.org/viewvc?view=revision&amp;revision=1370674 ------------------------------------------------------------------------ r1370674 | kmarsden | 2012-08-07 23:54:46 -0700 (Tue, 07 Aug 2012) | 3 lines DERBY-5896 Change java/drda source code to use spaces instead of tabs ------------------------------------------------------------------------ http://svn.apache.org/viewvc?view=revision&amp;revision=1371306 ------------------------------------------------------------------------ r1371306 | kmarsden | 2012-08-09 09:41:32 -0700 (Thu, 09 Aug 2012) | 5 lines DERBY-5896 Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client Change client tabs to spaces. ------------------------------------------------------------------------ http://svn.apache.org/viewvc?view=revision&amp;revision=1372814 ------------------------------------------------------------------------ r1372814 | kahatlen | 2012-08-14 02:53:09 -0700 (Tue, 14 Aug 2012) | 4 lines DERBY-5896 : Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client Align with tab stops if the replaced tab character itself wasn't located on a tab stop. ------------------------------------------------------------------------ http://svn.apache.org/viewvc?view=revision&amp;revision=1372841 ------------------------------------------------------------------------ r1372841 | kahatlen | 2012-08-14 04:17:14 -0700 (Tue, 14 Aug 2012) | 4 lines DERBY-5896 : Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client Align with tab stops in java/client. ------------------------------------------------------------------------ http://svn.apache.org/viewvc?view=revision&amp;revision=1372818 ------------------------------------------------------------------------ r1372818 | kahatlen | 2012-08-14 03:05:00 -0700 (Tue, 14 Aug 2012) | 4 lines DERBY-5896 : Change java/drda source code to use spaces instead of tabs and clean up tab creep in java/client Align with tab stops if the replaced tab character itself wasn't located on a tab stop. ------------------------------------------------------------------------
        Hide
        Knut Anders Hatlen added a comment -

        Attaching a patch with a similar cleanup for the client code. Committed revision 1372841.

        Show
        Knut Anders Hatlen added a comment - Attaching a patch with a similar cleanup for the client code. Committed revision 1372841.
        Hide
        Knut Anders Hatlen added a comment -

        > As part of the original reformat, I included things like build.xml and package.html, actually passing `find java/drda -type f` rather than just the java files.

        In the non-java files, there was just a single occurrence of tab+space+tab being expanded to 9 spaces instead of 8 in build.xml. Fixed in revision 1372818.

        Show
        Knut Anders Hatlen added a comment - > As part of the original reformat, I included things like build.xml and package.html, actually passing `find java/drda -type f` rather than just the java files. In the non-java files, there was just a single occurrence of tab+space+tab being expanded to 9 spaces instead of 8 in build.xml. Fixed in revision 1372818.
        Hide
        Knut Anders Hatlen added a comment -

        Committed derby-5896-align-with-tab-stops.diff.txt with revision 1372814.

        Show
        Knut Anders Hatlen added a comment - Committed derby-5896-align-with-tab-stops.diff.txt with revision 1372814.
        Hide
        Kathey Marsden added a comment -

        Below are some tips for negotiating white space differences now that java/drda and java/client have been reformated to have only space indentation (DERBY-5896). The replacement of tabs with four spaces was done for java/drda at revision 1370673 and java/client at revison 1371306.

        svn merge, blame and diff commands (command line)
        Specify -x -b on these svn commands to ignore white space differences. I changed my svnmergefromtrunk script as follows and had no problem merging DERBY-1400 to 10.9, a post reformat change:
        #svnmergefromtrunk e.g for change 1371677 svnmergefromtrunk 1371677
        svn merge -x -b -c $1 https://svn.apache.org/repos/asf/db/derby/code/trunk

        visual diff tools.
        Most visual diff tools have options for ignoring white space diffs which should be set when comparing branches or old versions.

        Eclipse subclipse plugin

        Set preferences under Window (or Project properties for fine project tuning)
        Window->Preferences->Compare/Patch - Check ignore white space.
        Window > Preferences> Java -> Code Style -> Formatter -> Edit -> Tab Policy - Choose Spaces Only
        Window > Preferences> General-> Text Editors - Check Insert spaces for tabs, show whitespace characters.

        Eclipse subclipse annotation
        subclipse annotation does not allow you to specify -x -b. To look at history prior to the change, however, you can specify a "To version" which can be prior to the reformat and give the annotation prior to the large change. I think we could add a PRESPACE tag or some such to make this easier. For drda, this is 1370673, for client 1371306. Another option is to use subclipse annotation on an older branch like 10.9.

        Eclipse subclipse merge
        I think this will not work very well until subclipse provides a whitespace option. The only option I know is to use svn merge from the command line.

        Are there other things that folks should consider? I only tried with eclipse IDE and do not merge in the IDE. I am guessing subclipse merge will have issues. Tips for Netbeans other IDE's would be welcome.

        Show
        Kathey Marsden added a comment - Below are some tips for negotiating white space differences now that java/drda and java/client have been reformated to have only space indentation ( DERBY-5896 ). The replacement of tabs with four spaces was done for java/drda at revision 1370673 and java/client at revison 1371306. svn merge, blame and diff commands (command line) Specify -x -b on these svn commands to ignore white space differences. I changed my svnmergefromtrunk script as follows and had no problem merging DERBY-1400 to 10.9, a post reformat change: #svnmergefromtrunk e.g for change 1371677 svnmergefromtrunk 1371677 svn merge -x -b -c $1 https://svn.apache.org/repos/asf/db/derby/code/trunk visual diff tools. Most visual diff tools have options for ignoring white space diffs which should be set when comparing branches or old versions. Eclipse subclipse plugin Set preferences under Window (or Project properties for fine project tuning) Window->Preferences->Compare/Patch - Check ignore white space. Window > Preferences > Java -> Code Style -> Formatter -> Edit -> Tab Policy - Choose Spaces Only Window > Preferences > General-> Text Editors - Check Insert spaces for tabs, show whitespace characters. Eclipse subclipse annotation subclipse annotation does not allow you to specify -x -b. To look at history prior to the change, however, you can specify a "To version" which can be prior to the reformat and give the annotation prior to the large change. I think we could add a PRESPACE tag or some such to make this easier. For drda, this is 1370673, for client 1371306. Another option is to use subclipse annotation on an older branch like 10.9. Eclipse subclipse merge I think this will not work very well until subclipse provides a whitespace option. The only option I know is to use svn merge from the command line. Are there other things that folks should consider? I only tried with eclipse IDE and do not merge in the IDE. I am guessing subclipse merge will have issues. Tips for Netbeans other IDE's would be welcome.
        Hide
        Kathey Marsden added a comment -

        As part of the original reformat, I included things like build.xml and package.html, actually passing `find java/drda -type f` rather than just the java files.

        Show
        Kathey Marsden added a comment - As part of the original reformat, I included things like build.xml and package.html, actually passing `find java/drda -type f` rather than just the java files.
        Hide
        Knut Anders Hatlen added a comment -

        Here's the command I used:

        for i in $(find java/drda -name "*.java"); do
        mv $i $i.orig
        expand -t4 < $i.orig > $i
        rm $i.orig
        done

        Show
        Knut Anders Hatlen added a comment - Here's the command I used: for i in $(find java/drda -name "*.java"); do mv $i $i.orig expand -t4 < $i.orig > $i rm $i.orig done
        Hide
        Kathey Marsden added a comment -

        Thanks Knut for fixing this up. Can you cut and paste your exact expand command for future reference if we do other code or need to do periodic cleanup?

        Show
        Kathey Marsden added a comment - Thanks Knut for fixing this up. Can you cut and paste your exact expand command for future reference if we do other code or need to do periodic cleanup?
        Hide
        Knut Anders Hatlen added a comment -

        Attaching derby-5896-align-with-tab-stops.diff.txt which changes the lines where the previous patch didn't hit the tab stop. Running regression tests.

        Show
        Knut Anders Hatlen added a comment - Attaching derby-5896-align-with-tab-stops.diff.txt which changes the lines where the previous patch didn't hit the tab stop. Running regression tests.
        Hide
        Knut Anders Hatlen added a comment -

        Sorry for commenting so late. Using sed to replace tabs with exactly four spaces won't do the right thing in all cases, as there may be less than four characters up to the next tab stop. A tool such as expand (part of GNU coreutils) would handle the cases where the tab is not located exactly at a tab stop. But I guess in most of the cases the script will produce ok results, as most of the tabs are at the beginning of the line. If some lines end up oddly formatted, we can reformat them as we come across them. Thanks for making this change.

        Show
        Knut Anders Hatlen added a comment - Sorry for commenting so late. Using sed to replace tabs with exactly four spaces won't do the right thing in all cases, as there may be less than four characters up to the next tab stop. A tool such as expand (part of GNU coreutils) would handle the cases where the tab is not located exactly at a tab stop. But I guess in most of the cases the script will produce ok results, as most of the tabs are at the beginning of the line. If some lines end up oddly formatted, we can reformat them as we come across them. Thanks for making this change.
        Hide
        Kathey Marsden added a comment -

        I have made the changes for this issue, but am leaving it open for a bit while I explore options to mitigate the large diff for annotation, merges, etc.

        In eclipse, subclipse annotate (right click, team -> Show annotation) does not currently have an option to ignore white space. You can however see older annotation without switching to 10.8. Instead of choosing the default to Revision: HEAD, I can choose revision 1370673 and see the older revision history. It might be useful to have a label for the prereformat so folks don't have to remember a number.

        There is a subclipse issue filed: http://subclipse.tigris.org/issues/show_bug.cgi?id=1200 but looks stuck as javaHL does not expose these options.

        Show
        Kathey Marsden added a comment - I have made the changes for this issue, but am leaving it open for a bit while I explore options to mitigate the large diff for annotation, merges, etc. In eclipse, subclipse annotate (right click, team -> Show annotation) does not currently have an option to ignore white space. You can however see older annotation without switching to 10.8. Instead of choosing the default to Revision: HEAD, I can choose revision 1370673 and see the older revision history. It might be useful to have a label for the prereformat so folks don't have to remember a number. There is a subclipse issue filed: http://subclipse.tigris.org/issues/show_bug.cgi?id=1200 but looks stuck as javaHL does not expose these options.
        Hide
        Kathey Marsden added a comment -

        Committed revision 1371306 for client patch.

        Show
        Kathey Marsden added a comment - Committed revision 1371306 for client patch.
        Hide
        Kathey Marsden added a comment -

        Attaching client patch for tabs that have crept into 36 files.

        Show
        Kathey Marsden added a comment - Attaching client patch for tabs that have crept into 36 files.
        Hide
        Kathey Marsden added a comment -

        java/client seems to have some tabs that have crept in. I will fix those up too as part of this issue so all the network code is spaces.

        Show
        Kathey Marsden added a comment - java/client seems to have some tabs that have crept in. I will fix those up too as part of this issue so all the network code is spaces.
        Hide
        Kathey Marsden added a comment -

        ommitted revision 1370674 to trunk.

        Show
        Kathey Marsden added a comment - ommitted revision 1370674 to trunk.
        Hide
        Kathey Marsden added a comment -

        With all of the cleanup Knut has been doing in the drda source and the fact that client is all spaces it seemed like a good time to do this for at least drda even if it will make merges hard. Hopefully though merges will be easy with the -x -b option. I couldn't really test this without checking in, so thought I would start with drda which I think is worth it even if it does cause merge pain points and see how merging a few changes goes. If merges are not problematic with -x -b then I am totally in favor of doing the whole thing.

        Also I am just taking it slow so everyone is comfortable. I remember a failed attempt at reformatting some years ago. I realize now, tabs were the real problem as other formatting inconsistencies are easier to negotiate. I hoping we can eliminate the tabs now, starting with this patch.

        Show
        Kathey Marsden added a comment - With all of the cleanup Knut has been doing in the drda source and the fact that client is all spaces it seemed like a good time to do this for at least drda even if it will make merges hard. Hopefully though merges will be easy with the -x -b option. I couldn't really test this without checking in, so thought I would start with drda which I think is worth it even if it does cause merge pain points and see how merging a few changes goes. If merges are not problematic with -x -b then I am totally in favor of doing the whole thing. Also I am just taking it slow so everyone is comfortable. I remember a failed attempt at reformatting some years ago. I realize now, tabs were the real problem as other formatting inconsistencies are easier to negotiate. I hoping we can eliminate the tabs now, starting with this patch.
        Hide
        Dag H. Wanvik added a comment -

        I'm +1 to this change. Just a question.. why are we doing it only for this part of the source?

        Show
        Dag H. Wanvik added a comment - I'm +1 to this change. Just a question.. why are we doing it only for this part of the source?
        Hide
        Kathey Marsden added a comment -

        Marking patch available. suites.All derbyall passed

        Show
        Kathey Marsden added a comment - Marking patch available. suites.All derbyall passed
        Hide
        Kathey Marsden added a comment -

        attaced is a patch for this issue.
        Using the script below as untab:
        #!/bin/bash
        files=$@
        for file in $files

        { echo $file mv $file $file.orig sed -e 's/\t/ /g' < $file.orig > $file rm $file.orig }

        from ../trunk/java/drda
        $ untab `find . -type f`

        Show
        Kathey Marsden added a comment - attaced is a patch for this issue. Using the script below as untab: #!/bin/bash files=$@ for file in $files { echo $file mv $file $file.orig sed -e 's/\t/ /g' < $file.orig > $file rm $file.orig } from ../trunk/java/drda $ untab `find . -type f`

          People

          • Assignee:
            Kathey Marsden
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development