search

OfficeDev / script-lab

Create, run and share your code directly from Office
MIT License
697 stars 157 forks source link

Copy to clipboard should honor tab size settings for entire yaml file #646

Open AlexJerabek opened 5 years ago

AlexJerabek commented 5 years ago

Bug Report

Expected behavior:

Copying the snippet to the clipboard should have a consistent tab size for the entire file. This includes the tabs before "content" lines.

Actual behavior:

The SL-default of 2 spaces is applied to the yaml, except for user-generated content.

Steps to Reproduce:

  1. Set "editor.tabSize" to 4
  2. Run formatting tool.
  3. Export a snippet by copying it to the clipboard.
  4. Paste it in VS Code.

Screenshot:

spaces

Zlatkovsky commented 5 years ago

Don't know if I fully understand the request -- but in any case, since it's not critical, putting it into the 1.3 milestone to review later.

Zlatkovsky commented 5 years ago

Also, @AlexJerabek , if you'd like to submit your own fix, by all means feel free :-)

nico-bellante commented 5 years ago

The culprit here is that the YAML.dumpSafe is not being passed the tabSize. (assuming they have a parameter you can pass for tabSize similar to JSON.stringify)

Zlatkovsky commented 5 years ago

That's the part I'm not sure about, is whether it even makes sense for the rest of the YAML to honor those tab sizes. Alex, let's talk in person at some point and see if you can convince me :-)

nico-bellante commented 5 years ago

I think the argument for it should is much stronger than it shouldn't. The main reason I see is it looks strange to see 2 space tabs and 4 space tabs together.

AlexJerabek commented 5 years ago

Aside from looking strange, it messes with VS Code's tab size autodetect.

I understand this is low priority (unless customers are using the yaml in similar manners as the content team). Just reporting an issue as it's encountered.

Zlatkovsky commented 5 years ago

Well, YAML spec allows for either 2 spaces or 4 spaces, so we're OK on that front. But what if someone chooses 3 spaces?...

That said, fine. We can have the logic use "editor.tabSize" if it's 2 or 4, and default to 4 otherwise. The safeDump method accepts an "indent" option (see more at https://www.npmjs.com/package/js-yaml). So it should be fairly trivial to fix