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: Change for file save on split editor when tabs are moved #5784

Merged
merged 2 commits into from
Nov 19, 2017

Conversation

csabella
Copy link
Contributor

@csabella csabella commented Nov 17, 2017

Fixes #5703.

This affected save(), save_as(), and save_copy_as() directly. Also added changes for set_todo_results() and set_analysis_results() as they were affected by the issue of using the same index across editor stacks.

@pep8speaks
Copy link

pep8speaks commented Nov 17, 2017

Hello @csabella! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 19, 2017 at 13:57 Hours UTC

@ccordoba12 ccordoba12 added this to the v3.2.5 milestone Nov 18, 2017
@@ -1690,8 +1780,6 @@ def save_copy_as(self, index=None):
txt = to_text_string(finfo.editor.get_text_with_eol())
try:
finfo.encoding = encoding.write(txt, filename, finfo.encoding)
self.file_saved.emit(str(id(self)), index, filename)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ccordoba12 The copy as function creates a new file with the same information as the current index, but it doesn't make any changes to the current index or current filename (in the tab or the self.data). The file_saved.emit was updating the other split panels for the current index/filename, but this isn't necessary since it's not changed at all.

It's simple to recreate to see what's happening, even without moving tabs.

  1. Open one file in one panel.
  2. Split horizontally or vertically.
  3. Click save_copy_as and save the file under a new name. You'll see the new file open in both panels, so there are now 2 tabs.
  4. Click focus onto the other panel.
  5. Notice that both tabs get updated with the same file name and no longer match the first panel.

This one really isn't related to moving tabs, but I found it while testing my changes. If you want, I can move it to a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

Don't worry, it's really good that you have detected and solved this problem as part of this PR.

@ccordoba12
Copy link
Member

Thanks for your contribution @csabella! It looks good to me, except for a minor comment I left. Please also fix the style issues reported by pep8speaks.

@ccordoba12
Copy link
Member

I think the reason why Pierre (Spyder's original author) didn't implement the functionality to move tabs in the Editor before was because of all the issues that would arise with split panes.

But thanks to the great work of Cheryl and @jnsebgosselin, now this is a thing from the past!!

@ccordoba12 ccordoba12 merged commit 44ddc68 into spyder-ide:3.x Nov 19, 2017
ccordoba12 added a commit that referenced this pull request Nov 19, 2017
@csabella csabella deleted the issue5703 branch November 20, 2017 00:33
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.

3 participants