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.

Sign up for GitHub

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

Jump to bottom

fix history panel not rendering when history is empty #1215

Merged
merged 1 commit into from
Feb 15, 2023
Merged

fix history panel not rendering when history is empty #1215

merged 1 commit into from
Feb 15, 2023

Conversation

basokant
Copy link
Contributor

@basokant basokant commented Feb 7, 2023

Provides fix for #1088

Freshly initialized repository after changes:

1088-fix

@welcome
Copy link

welcome bot commented Feb 7, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@github-actions
Copy link

github-actions bot commented Feb 7, 2023

Binder 👈 Launch a binder notebook on branch basokant/jupyterlab-git/fix-issue-1088

@fcollonval fcollonval added the bug label Feb 7, 2023
fcollonval
fcollonval requested changes Feb 7, 2023
View reviewed changes
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 and congrats for your first PR.

There is still an issue when you

  1. open an existing git repository
  2. open the history tab for that repository
  3. open a folder outside of any git repository
  4. make the folder a git repository through the extension (you can do that by selected the folder in the file browser, then go to the git extension panel and press the Initialize repo button)
  5. open the history for that new repository
  6. You should see the history for the other repository

The error seen in the console is

image

The fix for that would be to catch the error when calling log in model.ts to return a empty list of commits.

https://github.com/jupyterlab/jupyterlab-git/blob/master/src/model.ts#L917

Let me know if you need more pointers

@basokant basokant requested a review from fcollonval February 7, 2023 19:14
fcollonval
fcollonval reviewed Feb 8, 2023
View reviewed changes
src/model.ts Outdated
}
);
} catch (error) {
return { code: 0, commits: [] };
Copy link
Member

Choose a reason for hiding this comment

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

As the code in the panel is:

jupyterlab-git/src/components/GitPanel.tsx

Lines 264 to 270 in c7e7363

let pastCommits = new Array<Git.ISingleCommitInfo>();
if (logData.code === 0) {
pastCommits = logData.commits;
}
this.setState({
pastCommits: pastCommits

The best is to return a non-null code (1 e.g) and the state will be an empty array as wanted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed this change! Thanks for the guidance @fcollonval!

@basokant basokant requested a review from fcollonval February 8, 2023 16:02
fcollonval
fcollonval approved these changes Feb 9, 2023
View reviewed changes
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

Could you confirm me you work with @shawnesquivel? If yes, I will add him as co-author.

@basokant
Copy link
Contributor Author

basokant commented Feb 9, 2023

Thanks a lot

Could you confirm me you work with @shawnesquivel? If yes, I will add him as co-author.

Yes @shawnesquivel and I both worked on the initial fix, but he wasn't able to submit the PR due to an issue with running the pre-commit hook, so I ended up submitting it.

@basokant @shawnesquivel @fcollonval
fix history panel not rendering when history is empty
5960aca 
catch error to handle the log return when there are no past commits in the current branch

prettify and format correctly

change handled return to simply a non-null, non-zero code rather than 0 with an empty list of commits

Rebases to include changes from #13645

Update Playwright Snapshots

Updates selection to selectionBackground, docs

Update Playwright Snapshots

Add renderer

Fix UI test locator

Fix linter

Update Playwright Snapshots

Buffer message while waiting for the terminal

Update Playwright Snapshots

Co-authored-by: Shawn Esquivel <[email protected]>
@fcollonval fcollonval merged commit 23e6786 into jupyterlab:master Feb 15, 2023
@welcome
Copy link

welcome bot commented Feb 15, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@fcollonval
Copy link
Member

@all-contributors please add @basokant for code

@allcontributors allcontributors bot mentioned this pull request Feb 15, 2023
Merged
@allcontributors
Copy link
Contributor

@fcollonval

I've put up a pull request to add @basokant! 🎉

@fcollonval
Copy link
Member

@all-contributors please add @shawnesquivel for code

@allcontributors allcontributors bot mentioned this pull request Feb 15, 2023
Merged
@allcontributors
Copy link
Contributor

@fcollonval

I've put up a pull request to add @shawnesquivel! 🎉

@fcollonval fcollonval mentioned this pull request Aug 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fcollonval fcollonval fcollonval approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@basokant @fcollonval