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.

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

Suggest to rename "Shell Command" to "Code Shell Command" #3334

Closed
weinand opened this issue Feb 23, 2016 · 14 comments
Closed

Suggest to rename "Shell Command" to "Code Shell Command" #3334

weinand opened this issue Feb 23, 2016 · 14 comments
Assignees
Labels
polish Cleanup and polish issue verified Verification succeeded
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Feb 23, 2016

On testing #3241:

In the Command Palette "Shell Command" sounds very generic and it might be not clear what's behind this. I suggest to change it to "Code Shell Command" or "Code Command Line Tool" (which would even work on Windows).

2016-02-23 15-22-50

@joaomoreno
Copy link
Member

I'm not sure adding Code would make it less generic... We don't have the best product name... :/

@joaomoreno
Copy link
Member

How does this sound?

System: Install shell command in PATH
System: Uninstall shell command from PATH

@egamma
Copy link
Member

egamma commented Feb 23, 2016

I suggest to make the description of the command more verbose:
Shell Command: Install command to launch Code in PATH

And Code would show in all the product flavors Code-insiders, Code-alpha.

@egamma
Copy link
Member

egamma commented Feb 23, 2016

May be even leave out the category 'System' or 'Shell Command'

@weinand
Copy link
Contributor Author

weinand commented Feb 23, 2016

"Install 'code-alpha' command line tool in PATH"

@egamma
Copy link
Member

egamma commented Feb 23, 2016

Better, but I don't like command line tool. This isn't really a tool but a command to launch code.

"Install `code-alpha' launch command in PATH"

@joaomoreno
Copy link
Member

How about

Install 'code-alpha' command in PATH

@egamma
Copy link
Member

egamma commented Feb 23, 2016

👍

@joaomoreno joaomoreno added this to the Feb 2016 milestone Feb 23, 2016
@joaomoreno
Copy link
Member

SOLD!

@joaomoreno joaomoreno added the polish Cleanup and polish issue label Feb 23, 2016
@joaomoreno
Copy link
Member

Unfortunately I can't do exactly this since at the point I require it I don't know whether the shortcut will be called code, code-alpha or code-insiders.

I will name the action Install 'code' command in PATH and simply inform the user that it is in fact code-alpha after it has been installed.

@bpasero The reason for this lies on the need to provide the label to the SyncActionDescriptor constructor. At that time, I don't know the name of the shortcut, since I get it from the context service.

@weinand
Copy link
Contributor Author

weinand commented Feb 24, 2016

@joaomoreno sounds good!

@joaomoreno joaomoreno assigned weinand and unassigned joaomoreno Feb 26, 2016
@weinand weinand added the verified Verification succeeded label Feb 26, 2016
@bpasero
Copy link
Member

bpasero commented Feb 29, 2016

@joaomoreno descriptors are static enough that you cannot create them if you need services. the point of F1 is to show descriptors and not instantiated actions.

@joaomoreno
Copy link
Member

I get it. Irrelevant to this particular issue, I would actually like to argue against using descriptors. Here's what I observe:

With descriptors:

  • We end up having more code overall
  • All that code is loaded into memory on startup, since we're talking about sync descriptors. That is the action, its descriptor, the descriptor registry, and the action cache. All of that gets loaded into memory.
  • An action, in its essence, is a simple JS object. The descriptor is another. You end up having 2 objects in memory. They last forever.
  • We seem to optimize for time by not calling all the instantiation services into the actions before they are actually needed. We end up allocating twice the memory we need. It's the trade-off.

Without descriptors:

  • All the code you need to write is the action itself
  • Only the action code is loaded into memory
  • The instantiation itself would happen at startup, possibly being intensive

I'm not really sure the performance benefits are so clear that the descriptors end up being the obvious choice. I might be wrong, but it would be a nice benchmark to test.

@bpasero
Copy link
Member

bpasero commented Feb 29, 2016

I am all up for not having sync descriptors at all, I was just describing our status quo.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
polish Cleanup and polish issue verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants