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

PR: Fix several code snippets corner cases #10256

Merged
merged 14 commits into from Sep 28, 2019

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Sep 19, 2019

Description of Changes

This PR addresses all of the code snippets errors reported on #10230

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #10289.
Fixes #10230

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:
@andfoy

@andfoy andfoy added this to the v4.0rc milestone Sep 19, 2019
@andfoy andfoy self-assigned this Sep 19, 2019
@andfoy
Copy link
Member Author

andfoy commented Sep 19, 2019

@ccordoba12 @dalthviz, does this PR adresses all of your errors with code snippets?

@ccordoba12
Copy link
Member

ccordoba12 commented Sep 19, 2019

Thanks @andfoy! There are still some things not covered by this PR:

  • Tab should not cycle among snippets but simply remove the snippet regions after reaching the last one and pressing Tab again.
  • When you manually remove the text of a snippet, I think the snippet region of that snippet should be removed too.

@ccordoba12
Copy link
Member

ccordoba12 commented Sep 19, 2019

A couple more issues:

  • Pressing Ctrl+Z or Crtl+X after editing a snippet doesn't restore the snippet positions. For example, in the screenshot below I removed the nc characters of func with backspace then press Ctrl+Z and got this result:

    Selección_017

  • If you manually move with the left/right arrow keys out of a snippet region, then all snippet regions should be removed. That's what VSCode does and I think it's a good approach.

@ccordoba12
Copy link
Member

ccordoba12 commented Sep 19, 2019

One more: if you enter the following code

from pandas import pd
pd.concat(objs)

and replace objs by as and other characters in quick sucession, then Spyder freezes (it's not always reproducible, but most of the time). After unfreeze it with a Ctrl+C in the terminal, I've got the following traceback related to snippets:

Traceback (most recent call last):
  File "/home/carlos/Projects/spyder/github-repo/spyder/plugins/editor/extensions/snippets.py", line 201, in _on_key_pressed
    self._process_text(text)
  File "/home/carlos/Projects/spyder/github-repo/spyder/plugins/editor/extensions/snippets.py", line 49, in wrapper
    return f(self, *args, **kwargs)
  File "/home/carlos/Projects/spyder/github-repo/spyder/plugins/editor/extensions/snippets.py", line 209, in _process_text
    self.insert_text(text, line, column)
  File "/home/carlos/Projects/spyder/github-repo/spyder/plugins/editor/extensions/snippets.py", line 335, in insert_text
    tokens = tokenize(text)
  File "/home/carlos/Projects/spyder/github-repo/spyder/utils/snippets/lexer.py", line 91, in tokenize
    if regex.match(temp_word) is not None:

@andfoy
Copy link
Member Author

andfoy commented Sep 19, 2019

Pressing Ctrl+Z or Crtl+X after editing a snippet doesn't restore the snippet positions. For example, in the screenshot below I removed the nc characters of func with backspace then press Ctrl+Z and got this result:

I'm very aware of this case, this is due to the Ctrl+Z behaviour of the CodeEditor, as opposed to the snippets one, while the first deletes the last sequence of characters, the latter stores a tree copy for n and c keypresses. Thus, when the undo call is applied, we backtrack to the "n" tree, rather to delete the "n" and "c" trees.

Is there any way to get the length of the undo text?

@ccordoba12
Copy link
Member

You'll have to look for it because I don't know, sorry.

@goanpeca
Copy link
Member

goanpeca commented Sep 20, 2019

@andfoy if I do the following

import json
json.dumps(123, 123)

# Entering snippet mode type 123 tab 123, then selecting all (while I still see the
# snippet squares) and delete everything I get
    self.insert_text(text, line, column)
  File "/Users/gpena-castellanos/Google Drive/develop/quansight/spyder/spyder/plugins/editor/extensions/snippets.py", line 339, in insert_text
    if node.name == 'EPSILON':
AttributeError: 'NoneType' object has no attribute 'name'

@andfoy
Copy link
Member Author

andfoy commented Sep 20, 2019

@andfoy if I do the following

Does this happens with this PR?

@goanpeca
Copy link
Member

Yes, it still happens qith this Pr

@andfoy
Copy link
Member Author

andfoy commented Sep 20, 2019

@goanpeca Could you please and try again?

@goanpeca
Copy link
Member

Sure! After lunch 🥗:-)

@goanpeca
Copy link
Member

@goanpeca Could you please and try again?

The latest fixes the issue @andfoy

@pep8speaks
Copy link

pep8speaks commented Sep 20, 2019

Hello @andfoy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 156:80: E501 line too long (80 > 79 characters)
Line 180:80: E501 line too long (80 > 79 characters)

Comment last updated at 2019-09-28 18:09:56 UTC

@andfoy
Copy link
Member Author

andfoy commented Sep 20, 2019

@ccordoba12 Does the last commit fixes all the undo behaviour?

@ccordoba12
Copy link
Member

@andfoy, I'm still seeing #10256 (comment) with this PR.

@andfoy
Copy link
Member Author

andfoy commented Sep 23, 2019

@ccordoba12 I cannot reproduce that one:

ezgif com-video-to-gif(12)

@ccordoba12
Copy link
Member

@andfoy, test_code_snippets is hanging now.

@andfoy
Copy link
Member Author

andfoy commented Sep 24, 2019

@ccordoba12 The azure test fail is unrelated to this one

@ccordoba12
Copy link
Member

@andfoy, please skip test_code_snippets on Windows, that could make our test suite pass there (I don't have time right now to analyze what's the problem that that test seems to be introducing).

@ccordoba12 ccordoba12 changed the title PR: Fix some code snippets corner cases PR: Fix several code snippets corner cases Sep 28, 2019
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @andfoy!! Great work here!

@ccordoba12 ccordoba12 merged commit 34041c0 into spyder-ide:master Sep 28, 2019
@andfoy andfoy deleted the snippets_stabilization branch September 28, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crash in 4.0.0b5 related to non-ascii characters and tooltips Several errors with code snippets
4 participants