Skip to content

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

Closed
Guy Waldman (guywaldman) wants to merge 3 commits into
microsoft:masterfrom
guywaldman:always-show-fix-and-test
Closed

Fix #58731 & add test & preserve input for "create new branch"#58963
Guy Waldman (guywaldman) wants to merge 3 commits into
microsoft:masterfrom
guywaldman:always-show-fix-and-test

Conversation

@guywaldman

@guywaldman Guy Waldman (guywaldman) commented Sep 19, 2018

Copy link
Copy Markdown
Contributor

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 Guy Waldman (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
Copy Markdown
Contributor Author

João Moreno (@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().

@guywaldman Guy Waldman (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 João Moreno (joaomoreno) added the git GIT issues label Sep 21, 2018
@joaomoreno João Moreno (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

João Moreno (joaomoreno) commented Jan 4, 2019

Copy link
Copy Markdown
Contributor

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

@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