Skip to content

fix: use updated Git API#24

Merged
mattrothenberg merged 1 commit intomainfrom
mr/fix-git-logic
Sep 6, 2022
Merged

fix: use updated Git API#24
mattrothenberg merged 1 commit intomainfrom
mr/fix-git-logic

Conversation

@mattrothenberg
Copy link
Copy Markdown
Contributor

As reported in #23, folks are having trouble opening Flat Editor and it's crashing with a Git error.

After digging into (and reproducing locally) the issue, it seems like the Git VSCode extension API changed somewhere along the way, and some of the methods we were using no longer returned the correct data.

I've added the latest and greatest Git type definitions from VSCode and removed a few unused paths in our VSCodeGit class

Comment thread package.json
Comment on lines +37 to +39
"extensionDependencies": [
"vscode.git"
],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While this isn't strictly necessary, I like calling out explicitly that we're using the native Git extension.

Comment thread src/flatConfigEditor.ts
await gitClient.waitForRepo(3)

// Next, let's grab the repo name.
await gitClient.waitForRepo()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We only ever call waitForRepo here... As such, I removed the "# of times" argument from the function and co-located it in the function implementation itself.

Comment thread src/git.ts
return new Promise((resolve, reject) => {
let interval = setInterval(() => {
try {
const remotes = this.repository._repository.remotes
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was no longer returning the right data

Comment thread src/types.ts
Comment on lines -122 to -125
export interface CommitOptions {
all?: boolean | 'tracked'
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Outdated type defs.

Copy link
Copy Markdown

@jaked jaked left a comment

Choose a reason for hiding this comment

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

approving on faith—cool to see a little of how it works!

@mattrothenberg mattrothenberg merged commit e47a1c7 into main Sep 6, 2022
@mattrothenberg mattrothenberg deleted the mr/fix-git-logic branch September 6, 2022 07:07
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.

2 participants