Add better support for checkout type config#97322
Add better support for checkout type config#97322joaomoreno merged 4 commits intomicrosoft:masterfrom
Conversation
extensions/git/src/commands.ts
Outdated
| results.push(...repository.refs.filter(ref => ref.type === RefType.Tag).map(ref => new CheckoutTagItem(ref))); | ||
| break; | ||
| default: | ||
| invalids.add(type); |
There was a problem hiding this comment.
I‘m not sure. Any suggestions?
There was a problem hiding this comment.
Dont need them. Consider my suggestion below which filters out the invalids in the reduce loop.
extensions/git/src/commands.ts
Outdated
| const checkoutType = config.get<string>('checkoutType') || 'local,remote,tags'; | ||
| const checkoutTypes = checkoutType.trim().split(',').map(type => type.trim()); |
There was a problem hiding this comment.
You can filter and unique the valid types in advance rather than collecting invalids in the loop.
| const checkoutType = config.get<string>('checkoutType') || 'local,remote,tags'; | |
| const checkoutTypes = checkoutType.trim().split(',').map(type => type.trim()); | |
| const checkoutTypeString = config.get<string>('checkoutType') || 'local,remote,tags'; | |
| const checkoutTypes = Array.from(new Set(checkoutTypeString.trim().split(',').map(type => type.trim()).filter(type => ['local','remote','tags'].indexOf(type) > 0))); |
| invalids.add(type); | ||
| break; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Instead of forEach loop try something:
const RefTypesByCheckoutTypes = { 'local': RefType.Head, 'remote': RefType.RemoteHead, 'tags': RefType.Tag};
const results: CheckoutItem[] = checkoutTypes.reduce((acc, type) => {
const items =
repository.refs
.filter(ref => ref.type === RefTypesByCheckoutType[ref.type])
.map(ref => new CheckoutItem(ref));
if(items.length>0) acc.push(...items);
return acc;
}, []);
Kingwl
force-pushed
the
better-checkout-type
branch
from
e3c7402 to
ae54053
Compare
|
Up. |
|
One of the suggestions @rehangit made is not implemented, and it did seem like a better and more readable solution than what is there now. |
| const checkoutTypeString = config.get<string>('checkoutType'); | ||
|
|
||
| const checkoutTypeOptions = ['local', 'remote', 'tags']; | ||
| const checkoutTypes = checkoutTypeString?.trim().split(',').map(type => type.trim()).filter(type => checkoutTypeOptions.includes(type)); |
There was a problem hiding this comment.
You can use [...new Set(checkoutTypes)] to filter the distinct values.
|
If my memory is correct. The problem is the |
joaomoreno
left a comment
joaomoreno
left a comment
There was a problem hiding this comment.
The JSON schema should be much more structured than that: we support arrays including duplicate validation. We should also still support the old string values, don't wanna break no one right? Finally, this is now O(R*T), if R is amount of refs and T is amount of ref types, it should just run on O(R) even if at expense of memory.
I've made those changes and merged this in! Thanks!
4 participants
This PR fixes #95385