fix-pymode-tab-space.dpatch

[G. Milde]

On 10.01.07, Jörg Sommer wrote:
> Hello G.,

> > My suggestion would be:
> > 
> > +private define py_whitespace(n)
> > +{
> > +   if ( get_blocal_var("py_use_tab") )
> > +      whitespace(n);
> > +   else
> > +      insert_spaces(n);
> > +}

> I change it.

Actually, it gets more complicated, as for consistency in existing
documents we need to override USE_TABS as well.


> > The choice of py_use_tab is non-intuitive and undocumented. 

> It's a bug fix and not intended for other usage.

Agreed. What I wanted to say is: the way the patched python_mode()
determines whether to use tabs or spaces for indentation (as stored in
py_use_tab) is non-intuitive and undocumented.

> > +   set_blocal_var(Py_Indent_Level == TAB, "py_use_tab");
> > 
> > With this logic, 
> > 
> > if I want to use tabs, I need to:
> > 
> >   add a python_mode_hook to set TAB to Py_Indent_Level or make sure

> Why your Py_Indent_Level is different from TAB if you want tabs?

e.g. because the upstream sets TAB from Py_Indent_Level, so users of
upstream pymode did not need to care about setting TAB accordingly.

However, the main point is "why should I want to use tabs if
Py_Indent_Level is not different from TAB"?

> > if I do not want to use tabs, I need to:
> > 
> >   make sure, that TAB_DEFAULT != Py_Indent_Level or add a
> >   python_mode_hook to set TAB to something different than Py_Indent_Level.

> This is not the intention of the patch. It tries not to introduce a new
> technology to define indention of Python files. 

However, the patch changes the way tab- versus space-indention is
determined beyond the fix of looking into existing files. 

TAB to SPACE

   Someone whith 
   
     variable Py_Indent_Level = 8;
     variable TAB_DEFAULT = 4;
    
   in jed.rc (and no pymode hook to set TAB individually) will get
   tab-indention without, but space-indention with the patch (for new
   files). (He/she could recognize the change by the new visible
   indention of 8 columns.)
   
   This change needs at least to be documented.  

SPACE to TAB

   Someone with

     variable TAB_DEFAULT = 4;

   in jed.rc (and no customization of Py_Indent_Level nor a pymode hook
   to set TAB individually) will get space-indention without, but
   tab-indention with the patch for new files. This change goes mostly
   unnoticed, as the visible indent by 4 columns remains.
   
   Most ugly: it introduces a mixture of tabs and spaces for existing
   files using spaces, as the initialization of py_use_tab is only
   corrected in the direction space->tab if a tab is found.

> The reporter of the bug complained about intermixed indention types in
> a file where one type (spaces or tabs) was used.

Well, then if we just want to fix the reported bug, 

 1. no change should be done to the initialization of tab- or
    space-indention for new files,

 2. python_mode() on a file containing indented code lines should
    override this default (in both directions!)

zu 1)

I agree that the old logic is flawed. Why should a Py_Indent_Level of 8
imply the wish to use tabs for indention. (I suppose the rationale was
that for Py_Indent_Level == 8, tabs can be used as this is what the
python interpreter expands tabs to.)

I cannot see how the current patch does improve in this point.
Instead of guessing the tab-vs.-space user intention from one variable
(without documenting the consequences of its setting), the patched
version complicates this decision by comparing two variables (one of them
is dependent on a third, if not set explicitely). 

I would either

 * keep the old (flawed) logic, 
 * always default to spaces (as this is the "official" Python
   recommendation), or
 * use Py_Indent_Level == 0 as indicator for tab use, with the explanation
 
   Py_Indent_Level == "Number of spaces to indent a code block"

   --> If I want to indent a block with tabs, I do not use any spaces
       (== 0) for this task.
   

zu 2)

While the patch does reduce the probability of mixed indention it doesnot
fix it completely:

* mixed use in existing code is not detected

* false-positives can switch to tab-indention even if the buffer contains
  valid space-indented code:
  
  1. TAB == Py_Indent_Level
  
  2. a hard tab in continuation lines like
  
     function(arg1, arg2,
     	      arg3)
  
  3. a hard tab indenting a literal string with ''' '''
  
I am not sure, whether we need to provide a "complete" patch. Fixing this
is not easy, I spent some days on a solution.

However, a partial solution should IMO 

  * bias towards space use,
  * not advertise itself as a fix of the bug.


BTW: while the tab-spaces problem is real, did you reproduce the bug with 
the original example?

>>>>
For example, type this script without ever using the tab key :

#! /usr/bin/env python

def test1(arg):
    # This line is indented using 4 spaces
    # This line too
    if 1 == 1:
        # However, this line is indented using one tab.
        pass

<<<<

With the default settings, the unpatched python_mode() will set
TAB = 0 and no tabs are used!

I could only reproduce this by setting back TAB = 8 in a hook.


Günter