Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect indentation after expanding python snippet #506

Closed
shaleh opened this issue Aug 15, 2014 · 14 comments
Closed

Incorrect indentation after expanding python snippet #506

shaleh opened this issue Aug 15, 2014 · 14 comments

Comments

@shaleh
Copy link

shaleh commented Aug 15, 2014

Emacs version: "GNU Emacs 24.2.1 (x86_64-apple-darwin, NS apple-appkit-1038.36)
of 2012-08-27"

I have yasnippet-20140729.1240 installed using cask.

My snippet looks like this:

# -*- mode: snippet -*-
# name: addlogging
# key: addlogging
# expand-var: ((yas-indent-line 'fixed))
# --
logger = logging.getLogger("")

if args.log_file:
    lh = logging.FileHandler(args.log_file)
else:
    lh = logging.StreamHandler()

lh.setFormatter(logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s"))

logger.addHandler(lh)

if args.debug:
    logger.setLevel(logging.DEBUG)
    args.verbose = True
elif args.verbose:
    logger.setLevel(logging.INFO)
else:
    logger.setLevel(logging.WARNING)

I added the yas-indent-line in expand-var to see if it helped. There is no difference with or without it.

When I expand the snippet this is what I get:

logger = logging.getLogger("")

if args.log_file:
    lh = logging.FileHandler(args.log_file)
else:
    lh = logging.StreamHandler()

    lh.setFormatter(logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s"))

    logger.addHandler(lh)

    if args.debug:
        logger.setLevel(logging.DEBUG)
        args.verbose = True
    elif args.verbose:
        logger.setLevel(logging.INFO)
    else:
        logger.setLevel(logging.WARNING)

Notice that most of the code is now aligned under the first else. This occurs when I run via emacs -Q, run the following to setup yasnippets, and then attempt to expand "addlogging" in a Python buffer:

(load "/Users/sean_perry/.emacs.d/site-lisp/_managed/24.2.1/cl-lib-0.5/cl-lib.el")
(load "/Users/sean_perry/.emacs.d/site-lisp/_managed/24.2.1/yasnippet-20140729.1240/yasnippet.el")
(yas-load-directory "/Users/sean_perry/.emacs.d/my-snippets")

I cannot say if this is new behaviour or not because I just made the snippet. None of the other Python snippets I have contain multiple if/else blocks.

Let me know how I can help debug this.

Thanks.

@shaleh
Copy link
Author

shaleh commented Aug 15, 2014

I just downloaded the latest Emacs for OSX which contains 24.3.1. Same problem.

I am using the included python.el.

@npostavs
Copy link
Collaborator

I added the yas-indent-line in expand-var to see if it helped. There is no difference with or without it.

You'll be wanting expand-env, don't forget to reload the snippet.

Maybe we should have a warning about unrecognized headers.

@joaotavora
Copy link
Owner

@npostavs, when and where would the warning be posted?

  • when: on snippet load versus on snippet expansion? The first is easier but the second is probably more useful and visible.
  • where: use display-warning with yasnippet category or something? Or just use yas-message?

It could also be an error on snippet load... but that would probably do more harm than good

@joaotavora
Copy link
Owner

By the way, this is related to #485. My conclusion there was that this is not an error, or at least it's Emacs behaviour by default.

This is still arguable, but I'm not so sure right now. I did some tests after pasting the snippet's body in a python-mode buffer to test Emacs's own behaviour:

  • M-x indent-region leaves the text untouched, as @shaleh intends.
  • going line by line and calling indent-for-tab-command produces the same error as yasnippet.
  • repeating the previous text with return added after the first else branch does not produce the error

I don't remember my own rationale to have the autoindent behaviour in yasnippet not use indent-region, though I suspect it was something important related to field overlays and such. Perhaps the issue could be revisited. Anyway, marking this a workaround.

@npostavs
Copy link
Collaborator

I think display-warning at load time.

Doing it on snippet expansion brings up the question of whether we want to warn every time the snippet is expanded or maybe just the first time. Plus, the snippet doesn't remember which file it's loaded from, right? Having a link to the line that causes the warning would be nice.

And display-warning is better than yas-message becaus the warning will be too easily go unnoticed in the *Messages* buffer.

@shaleh
Copy link
Author

shaleh commented Aug 18, 2014

Thanks, yes expand-env makes it work how I intend.

Is there a way to enable this setting for all Python buffers?

@npostavs
Copy link
Collaborator

Is there a way to enable this setting for all Python buffers?

See #485 (comment)

@shaleh
Copy link
Author

shaleh commented Aug 18, 2014

See my comment on #485. It looks like yasnippets combined with the new Python mode yields incorrect behavior for anything involving indentation choices. Even the simple try/except clause snippet is improperly indented. You might want to consider enabling this workaround whenever the new Python mode is detected. Or, at the very least making a prominent note in the documentation that Python users will want to add the hook documented in #485.

@joaotavora
Copy link
Owner

@npostavs what do you think of taking @shaleh's suggestion and adding to python-mode-hook in a .yas-setup.el file in the snippets repo?

@shaleh
Copy link
Author

shaleh commented Aug 18, 2014

If you can provide tests you would like me to run, I will gladly run them.

I also dabble in Haskell and Idris which are also whitespace sensitive. I see similar misbehavior there if I do not apply the same expand-env trick. I have not had the chance to do an exhaustive emacs -Q test yet. More when I have.

@npostavs
Copy link
Collaborator

adding to python-mode-hook in a .yas-setup.el file in the snippets repo?

That seems sensible. It was discussed in AndreaCrotti/yasnippet-snippets#34, but things sort of died out there without anything concrete happening.

@joaotavora
Copy link
Owner

@shaleh, the tests I'm asking for are simple.

Everytime you discover yasnippet indenting behaviour that you find is surprising/irritanting/wrong, exit the snippet with some dummy values for the fields. Then, without ever re-entering yasnippet, call indent-for-tab-command (should be TAB) in each line of the ex-snippet, starting from the first. If you get the same feeling of suprise/irritation/wrongness, that's probably a hint that either Emacs's own behaviour can be fixed (or perhaps that it can already be customized).

@shaleh
Copy link
Author

shaleh commented Aug 19, 2014

Thank you for the explanation @capitaomorte. I shall do so in the future.

@shaleh
Copy link
Author

shaleh commented Aug 20, 2014

More testing.

python-mode does not show the problem. This is a python.el bug for sure. I just started using python.el after updating my Emacs so I had not noticed the little differences yet.

In a python-mode buffer:

try:<enter>
    <enter>
    except<tab>

The except lands back at the beginning of the line. No amount of pressing tab will cause it to move :-)

In python.el:

try:<enter>
<enter>
except<tab>

Here you are not immediately indented out 4 spaces after pressing enter following the try. When you press tab after type 'except' the whole like is indented 4 spaces. Pressing tab again moves it back to column 0. You can repeat the indent cycle by repeatedly pressing tab. Sigh. This means debugging the python.el code and dealing with the GNU emacs maintainers to get it fixed in the long run.

Short term I am switching back to python-mode which has always worked for me.

I still need to look into the Haskell and Idris modes though.

Based on what I have found so far it looks like setting certain modes to always be in 'fixed' mode is probably the right choice. Very few snippets will work correctly in python.el.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants