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 syncing IPython console cwd with the Working directory toolbar #19068

Merged
merged 10 commits into from
Aug 18, 2022

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Aug 15, 2022

Description of Changes

  • This was working before but I don't know when it broke. Now it works in three ways:
    1. When switching between consoles.
    2. When opening new consoles.
    3. When opening/closing projects.
  • Add tests to check that the Working directory toolbar and Files are updated in those cases.
  • Set the right working directory when opening a console from Files (this was reported on issue IPython ignore "Open IPython console here" #19003).
  • Restore functionality of IPython console's public API method set_working_directory, which I broke in PR PR: Add new API methods to the IPython console #18544.
  • Add a new method to it called save_working_directory to save the cwd reported by the Working directory plugin. This is necessary to start new consoles.
  • Rename some methods related to the cwd in the IPython console for clarity.

Issue(s) Resolved

Fixes #19003

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: @ccordoba12

@ccordoba12 ccordoba12 added this to the v5.3.3 milestone Aug 15, 2022
@ccordoba12 ccordoba12 self-assigned this Aug 15, 2022
@ccordoba12 ccordoba12 changed the title PR: Sync IPython console cwd with the Working directory toolbar PR: Fix syncing IPython console cwd with the Working directory toolbar Aug 15, 2022
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @ccordoba12 ! Checking locally seems like there is an odd behavior when opening a project. Seems like the working directory is not being updated and then any new consoles start with the previous working directory that was set before opening the project:

cwd

Also, when closing the project the working directory still references the project path but then the new console cwd is not set to be the last path but the default user directory?:

imagen

- Those are created not only when Spyder is started but also when
opening/closing projects.
- Expand tests to check that cwd is synced when opening/closing
projects.
@ccordoba12
Copy link
Member Author

ccordoba12 commented Aug 17, 2022

Thanks for checking those cases @dalthviz! I think I fixed them in my last commit, in which I also expanded the new tests I added to cover them.

@dalthviz
Copy link
Member

Thanks for the changes @ccordoba12 ! Seems like the first part is fixed However, the last part (when closing a project the old working directory set is changed to be again the user default/home directory) is still failing. Locally, I can see that when closing a project the path in the working directory toolbar is the correct one for an split second but then after the console finishes loading it changes to use the user default/home directory:

cwd

@ccordoba12
Copy link
Member Author

Locally, I can see that when closing a project the path in the working directory toolbar is the correct one for an split second but then after the console finishes loading it changes to use the user default/home directory

This is the default behavior @dalthviz, i.e. when closing a project the first console's cwd is set to the user's home directory. What would you expect it to be? The cwd before the project was open?

@dalthviz
Copy link
Member

What would you expect it to be? The cwd before the project was open?

Yep exactly

@ccordoba12
Copy link
Member Author

ccordoba12 commented Aug 17, 2022

Ok, that sounds like a reasonable expectation when there's a single console open (as in the gif you posted above). However, if users have many of them, we would have to use as the "previous cwd" the one of the console which has focus before the project is open.

And in that case, I don't know if users would be surprised by us setting the cwd when the project is closed to what could appear to be a random directory, specially if they forgot what was the cwd of the last console with focus.

@dalthviz
Copy link
Member

I see, also if the current behavior is the default behavior then I guess this is ok 👍 . Maybe creating a new issue to discuss that behavior in the ux-improvements repo could be worthy, but for sure not something to address here. So, besides the comment I left for the update_in_spyder new kwarg, this LGTM 👍

Also expand the docstring of that method.
@ccordoba12
Copy link
Member Author

Maybe creating a new issue to discuss that behavior in the ux-improvements repo could be worthy

Good idea. I opened spyder-ide/ux-improvements#74 to keep track of this discussion.

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @ccordoba12 👍

@dalthviz dalthviz merged commit 40216aa into spyder-ide:5.x Aug 18, 2022
dalthviz added a commit that referenced this pull request Aug 18, 2022
@ccordoba12 ccordoba12 deleted the issue-19003 branch August 18, 2022 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants