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

Refresh open file on pull #1090

Merged
merged 2 commits into from
Mar 21, 2022
Merged

Conversation

ajbozarth
Copy link
Member

Currently doing a pull does not update the contents of open files
like switching branches does.

This runs the same function to update files as switching branches but instead of diffing the branches it diffs the previous top commit to the new HEAD.

Fixes #1086

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Currently doing a pull does not update the contents of open files
like switches branches does.

Fixes jupyterlab#1086
@ajbozarth ajbozarth requested a review from fcollonval March 15, 2022 22:36
@ajbozarth ajbozarth self-assigned this Mar 15, 2022
@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch ajbozarth/jupyterlab-git/refresh_on_pull

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @ajbozarth

There is a bug in the existing method. And the call order needs to be set up a bit differently to ensure we get the correct value.

  1. Store the top commit hash as we don't control its update - it may happen after the pull and before the changed files are requested.

  2. We should wait for the task

const data = await this._taskHandler.execute
  1. Get the changed files and revert

  2. await for this.refreshBranch()

@fcollonval
Copy link
Member

Looking at the test failure, we need to check if _currentBranch is null or not 😉

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@ajbozarth
Copy link
Member Author

@fcollonval I made the updates you suggested. I original had the top commit stored first, but I stream-lined my changes when I found that the refreshBranch function call is what updated it. I do believe storing it first is still the safer plan though, even with adding the awaits

Copy link
Member

@fcollonval fcollonval 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 @ajbozarth

@fcollonval fcollonval merged commit cca3808 into jupyterlab:master Mar 21, 2022
@ajbozarth ajbozarth deleted the refresh_on_pull branch March 21, 2022 18:13
@ajbozarth
Copy link
Member Author

@fcollonval Thanks for the merge, given I have users waiting on this fix what do you think the timeline is on releasing it? It looks like master has at least one feature merged since the last patch release so I assume the next release would have to be a minor release. In which case would we be waiting for the couple of recent open PRs to merge first?

@fcollonval
Copy link
Member

Release is in progress @ajbozarth

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.

Automatic reload from disc after git pull
2 participants