Skip to content

Fix #58731 & add test & preserve input for "create new branch"#58963

Closed
guywaldman wants to merge 3 commits intomicrosoft:masterfrom
guywaldman:always-show-fix-and-test
Closed

Fix #58731 & add test & preserve input for "create new branch"#58963
guywaldman wants to merge 3 commits intomicrosoft:masterfrom
guywaldman:always-show-fix-and-test

Conversation

@guywaldman
Copy link
Contributor

@guywaldman guywaldman commented Sep 19, 2018

After #58731, when explicitly setting items on a QuickPick (i.e. via the createQuickPick() API as opposed to showQuickPick() with the items as arguments), the shouldAlwaysShow field was not propagated.

This PR resolves this issue and adds a test.

@guywaldman guywaldman changed the title Fix #58731 & add integration test Fix #58731 & integration test & preserve input for "create new branch" Sep 20, 2018
@guywaldman
Copy link
Contributor Author

@joaomoreno

Added a commit that adds preserving the input after accepting the "Create new branch" option (as per your comment).

Had to change some things (commit description contains details), would appreciate a strict CR to verify that it shouldn't break anything.

From my investigation, it appears that prior to the fix programmatically calling CommandCenter methods with the decorator @command(..., { repository: true }) and multiple parameters.
For instance calling this.deleteBranch(repository, 'someExistingBranch') through the debugging console would not pass on the right arguments due to the logic in .createCommand().

@octref octref requested a review from joaomoreno September 20, 2018 19:08
@guywaldman guywaldman changed the title Fix #58731 & integration test & preserve input for "create new branch" Fix #58731 & add test & preserve input for "create new branch" Sep 20, 2018
@joaomoreno joaomoreno added the git GIT issues label Sep 21, 2018
@joaomoreno joaomoreno added this to the Backlog milestone Sep 21, 2018
* When setting items explicitly (i.e. using the `createQuickPick()` API),
the `shouldAlwaysShow` field will now also be propagated
* Added test to enforce proper behavior of `shouldAlwaysShow` field in `QuickPick` items
* Propagated input from `QuickPick` through `CreateBranchItem` to the `git.branch` command

* Entailed changing `.createCommand()` to not prepend the repository when its already the first argument (did not allow to call `CommandCenter` methods with multiple arguments with `repository` set to true in options)
@joaomoreno joaomoreno modified the milestones: Backlog, October 2018 Oct 1, 2018
@joaomoreno joaomoreno modified the milestones: October 2018, November 2018 Nov 6, 2018
@joaomoreno joaomoreno modified the milestones: November 2018, Backlog Dec 3, 2018
@joaomoreno
Copy link
Member

joaomoreno commented Jan 4, 2019

This actually is #60895

I'll close this since as I merged it in, the refactorings were just too big that I ended up rewriting it. Sorry about that. Thanks anyway! 🍻

dcb3c33

@joaomoreno joaomoreno closed this Jan 4, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

git GIT issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants