bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

[Dima Kogan]

Hi. I've been using the recently-committed hide-show functionality from:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56635

It's working well, but I just found a problem. Python lacks explicit
block-end characters that a language like C has. This creates some
ambiguity about what is intended, and I think emacs makes the wrong
choice in at least one spot. Let's say we have a tst.py:

def f():
    if 1:
        11
    elif 2:
        22
    elif 3:
        33
    elif 4:
        44
    else:
        5

If I want to collapse the "elif 2:" block, I move the point to somewhere
inside that block, and (hs-hide-block). Placing the point at the end of
the "elif 2:" line puts us unambiguously inside that block, so this does
the job.

But what if the point is at the start of the "elif 2:" line? Currently
emacs considers this to be INSIDE this block also, so (hs-hide-block)
there also hides the "elif 2" block, but should it? Would it not make
more sense if the START of the "elif 2:" line was considered in the "def
f():" block, but outside all the "if" blocks? Then (hs-hide-block) at
the start of that line would collapse the whole "def f():" block.
Currently the only point where this can be done is at the end of the
"def f():" line.

This also raises a question about what should happen if we
(hs-hide-block) with the point on the "i" in any of the "elif" lines.
Probably this should be outside the "elif" blocks too? Maybe?

Thanks!

bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

[kobarity]

Dima Kogan wrote:
> But what if the point is at the start of the "elif 2:" line? Currently
> emacs considers this to be INSIDE this block also, so (hs-hide-block)
> there also hides the "elif 2" block, but should it? Would it not make
> more sense if the START of the "elif 2:" line was considered in the "def
> f():" block, but outside all the "if" blocks? Then (hs-hide-block) at
> the start of that line would collapse the whole "def f():" block.
> Currently the only point where this can be done is at the end of the
> "def f():" line.

Hi.  I think current hideshow behavior is in line with block
navigation in Python mode.  For example, if the point is at the start
of the "elif 2:" line, M-x python-nav-beginning-of-block moves the
point to "e" of "elif 2:".  This implies that Emacs Python mode
considers spaces before "elif 2:" to belong to the "elif 2:" block.
So I think this is not a matter of hideshow, but of how the block
should be recognized.

The current implementation seems to assume that the block boundary is
always at the end of the line, as shown in the following figure.

_______________
|def f():      |
|_____________ |
||    if 1:   ||
||________11__||
||    elif 2: ||
||________22__||
|______________|

On the other hand, Dima seems to expect the block boundary to be as
shown in the following figure.

_______________
|def f():      |
|    _________ |
| ___|if 1:  _||
||_______ 11|_ |
| ___|elif 2:_||
||_______ 22|  |
|______________|

Although both models may have pros and cons, the current
implementation seems more natural to me.  In particular, it would be
more natural for "elif" or "else" block to be adjacent to "if" block
with nothing else in between.

Regards,

bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

[Andreas Röhler]

Am 04.09.22 um 17:15 schrieb kobarity:
> Dima Kogan wrote:
>> But what if the point is at the start of the "elif 2:" line? Currently
>> emacs considers this to be INSIDE this block also, so (hs-hide-block)
>> there also hides the "elif 2" block, but should it? Would it not make
>> more sense if the START of the "elif 2:" line was considered in the "def
>> f():" block, but outside all the "if" blocks? Then (hs-hide-block) at
>> the start of that line would collapse the whole "def f():" block.
>> Currently the only point where this can be done is at the end of the
>> "def f():" line.
> Hi.  I think current hideshow behavior is in line with block
> navigation in Python mode.  For example, if the point is at the start
> of the "elif 2:" line, M-x python-nav-beginning-of-block moves the
> point to "e" of "elif 2:".  This implies that Emacs Python mode
> considers spaces before "elif 2:" to belong to the "elif 2:" block.
> So I think this is not a matter of hideshow, but of how the block
> should be recognized.
>
> The current implementation seems to assume that the block boundary is
> always at the end of the line, as shown in the following figure.
>
> _______________
> |def f():      |
> |_____________ |
> ||    if 1:   ||
> ||________11__||
> ||    elif 2: ||
> ||________22__||
> |______________|
>
> On the other hand, Dima seems to expect the block boundary to be as
> shown in the following figure.
>
> _______________
> |def f():      |
> |    _________ |
> | ___|if 1:  _||
> ||_______ 11|_ |
> | ___|elif 2:_||
> ||_______ 22|  |
> |______________|
>
> Although both models may have pros and cons, the current
> implementation seems more natural to me.  In particular, it would be
> more natural for "elif" or "else" block to be adjacent to "if" block
> with nothing else in between.
>
> Regards,
>
>
> Hi,

IMO the if-block starts at column 4, with "if". It ends at any column 
lower than 4 below that number 22.

Assume everything between with no regard of column as part of that block.

Best,

Andreas

bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

[Dima Kogan]

Hi. Thanks for the analysis. I guess this isn't the right forum for this
discussion, but I'll put it here anyway. The reason I think the "if" and
"else" text and the preceding whitespace should be considered to lie
outside their block is that this is how cc-mode works. Consider the
C code equivalent to the Python code in the bug report:

void f(void)
{
    if(1)
    {
        11;
    }
    else if(2)
    {
        22;
    }
    else if(3)
    {
        33;
    }
    else if(4)
    {
        44;
    }
    else
    {
        5;
    }

    return 0;
}

Here (hs-hide-block) hides the if/else blocks ONLY if the point is
inside the {}. Otherwise, the whole f() is hidden.

bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

[Andreas Röhler]

Am 05.09.22 um 09:44 schrieb Dima Kogan:
> Hi. Thanks for the analysis. I guess this isn't the right forum for this
> discussion, but I'll put it here anyway. The reason I think the "if" and
> "else" text and the preceding whitespace should be considered to lie
> outside their block is that this is how cc-mode works. Consider the
> C code equivalent to the Python code in the bug report:
>
> void f(void)
> {
>      if(1)
>      {
>          11;
>      }
>      else if(2)
>      {
>          22;
>      }
>      else if(3)
>      {
>          33;
>      }
>      else if(4)
>      {
>          44;
>      }
>      else
>      {
>          5;
>      }
>
>      return 0;
> }
>
> Here (hs-hide-block) hides the if/else blocks ONLY if the point is
> inside the {}. Otherwise, the whole f() is hidden.
>
>
>

There is another point: might be more than one if-statement in a 
function. If the whole function-body is collapsed from just one 
statement, the others are invisible too.

bug#57551: 29.0.50; hide-show in python-mode selects the current block imperfectly

[kobarity]

Andreas Röhler wrote:
>> The current implementation seems to assume that the block boundary is
>> always at the end of the line, as shown in the following figure.
>>
>> _______________
>> |def f():      |
>> |_____________ |
>> ||    if 1:   ||
>> ||________11__||
>> ||    elif 2: ||
>> ||________22__||
>> |______________|
>>
>> On the other hand, Dima seems to expect the block boundary to be as
>> shown in the following figure.
>>
>> _______________
>> |def f():      |
>> |    _________ |
>> | ___|if 1:  _||
>> ||_______ 11|_ |
>> | ___|elif 2:_||
>> ||_______ 22|  |
>> |______________|
>>
>IMO the if-block starts at column 4, with "if". It ends at any column 
>lower than 4 below that number 22.

Hi.  Is it correct that you are expecting a single block as shown
below?

_______________
|def f():      |
|    _________ |
| ___|if 1:   ||
||        11  ||
||    elif 2: ||
||_______ 22__||
|______________|

If so, this is another topic.

Dima Kogan wrote:
> Hi. Thanks for the analysis. I guess this isn't the right forum for this
> discussion, but I'll put it here anyway. The reason I think the "if" and
> "else" text and the preceding whitespace should be considered to lie
> outside their block is that this is how cc-mode works. Consider the
> C code equivalent to the Python code in the bug report:
> 
> void f(void)
> {
>     if(1)
>     {
>         11;
>     }
>     else if(2)
>     {
>         22;
>     }
>     else if(3)
>     {
>         33;
>     }
>     else if(4)
>     {
>         44;
>     }
>     else
>     {
>         5;
>     }
> 
>     return 0;
> }
> 
> Here (hs-hide-block) hides the if/else blocks ONLY if the point is
> inside the {}. Otherwise, the whole f() is hidden.

I think this is reasonable for cc-mode, where blocks are explicitly
defined by curly braces only.  Keywords like "if" or "else" have
nothing to do with identifying blocks.  Also, there is no need to
consider how lines are constructed.

However, Python does not have such block boundary characters.  As
Python blocks are identified with keywords and indentation, it is
important how lines are constructed.  It seems to me that limiting the
block boundary to the end of a line is not a bad idea in Python mode.

Andreas Röhler wrote:
> There is another point: might be more than one if-statement in a 
> function. If the whole function-body is collapsed from just one 
> statement, the others are invisible too.

Isn't this an expected behavior when hiding the function? Could you
give an example and your expectation?

Regards,