Support "+ system clipboard register (#780)

[bdchauvette]

This adds support for accessing the system clipboard using the "+ register, addressing #780.

The "+ register is basically an alias for the "* register, so I didn't make any changes to how the useClipboard setting is handled.

[rebornix]

just a single tiny comment and we are good to go!

[bdchauvette]

@rebornix Awesome! Just let me know what part you'd like commented, and I'll add it when I get a chance 👍

[johnfn]

Thanks for the PR! Really nice work, just one small suggestion.

[johnfn]

Instead of this, make IRegisterContent have another optional field called isClipboardRegister and put the value on there.

[bdchauvette]

Good call! I've switched the check to look up the isClipboardRegister property on the register.

(For consistency's sake, I also renamed the check from Register.isSystemClipboardRegister to just Register.isClipboardRegister. Saves a few keystrokes too!)

[johnfn]

Can you just do return register && register.isClipboardRegister?

[bdchauvette]

I actually tried that initially, but tsc doesn't like it very much:

> vim@0.1.10 vscode:prepublish c:\Users\Ben\Code\vscode-vim
> node ./node_modules/vscode/bin/compile -p ./
src/register/register.ts(42,12): error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'.
  Type 'undefined' is not assignable to type 'boolean'.

It works fine if I change the signature to isClipboardRegister(string): boolean | undefined, but I wasn't sure if that was preferable to having a messier function body.

[johnfn]

Can you show me the exact line(s) of code that you're using to get that error?

[bdchauvette]

  public static isClipboardRegister(registerName: string): boolean {
    const register = Register.registers[registerName];
    return register && register.isClipboardRegister;
  }

I think it's being caused by the isClipboardRegister property being optional: the property has a type signature of boolean | undefined, but the Register.isClipboardRegister method expects to only return booleans.

If we try to return the property without first converting it to a boolean, it's possible for the value to undefined (b/c it's optional), which violates the signature for the checking method.

(I think) this would also explain why it works if I change the allowed return type to boolean | undefined.

I'm pretty new to typescript, so I'm not sure what the idiomatic way to handle this kind of thing is 😕

[bdchauvette]

Each of these allow the code to compile without a hitch:

// Cast to boolean at runtime
return register && Boolean(register.isClipboardRegister);

// or:
return register && !!register.isClipboardRegister;

// Type assertion
return register && (register.isClipboardRegister as boolean);

// No return type annotation
public static isClipboardRegister(registerName: string) {
  const register = Register.registers[registerName];
  return register && register.isClipboardRegister;
}

[johnfn]

Ohhhhh. It's because it's an optional property, which has type boolean | undefined, which we are returning directly. That makes sense... in a weird way. :)

I guess TypeScript is pointing out that we did a dumb thing by saying the type was isClipboardRegister?: boolean since now we can't distinguish undefined from false in an easy way.

Best is probably to change the type to isClipboardRegister: boolean, and add it to all registers. Then this code will work and also our undefined vs false case will go away.

TypeScript caught a (minor) bug! Yay, TypeScript!

[bdchauvette]

Yay, TypeScript! 🙌

Looks like I just broke Travis, though. Any ideas what's going on there? 😕

[jpoon]

¯_(ツ)_/¯ restarted the Travis build.

[johnfn]

Annnd it's good!

Thanks @bdchauvette for the PR! :)

[bdchauvette]

Awesome! Thanks for such a great project :D