search

SchoofsKelvin / vscode-sshfs

Extension for Visual Studio Code: File system provider using SSH
GNU General Public License v3.0
543 stars 36 forks source link

Simplify NewConfig description #285

Closed grtcdr closed 2 years ago

grtcdr commented 2 years ago

The current description is in my opinion longer than it should be. I propose compiling the accepted values into a list rather than describing them through words.

grtcdr commented 2 years ago

I also noticed some words were abbreviated in the config editor view, while others were not. In this case I'll refer you to two separate descriptions which both say the same thing about supporting ENV variables, but they say it differently. I suggest keeping the descriptions consistent with one another.

See:

https://github.com/SchoofsKelvin/vscode-sshfs/blob/48ef2298a8985c5748680d66477ea31c841f8592/webview/src/ConfigEditor/fields.tsx#L102

And:

https://github.com/SchoofsKelvin/vscode-sshfs/blob/48ef2298a8985c5748680d66477ea31c841f8592/webview/src/ConfigEditor/fields.tsx#L94

grtcdr commented 2 years ago

@SchoofsKelvin Would you like me to work on the other descriptions?

SchoofsKelvin commented 2 years ago

I'll go through the descriptions. It's true that most of them are more written from a technical standpoint than a consistent UI-wise one. You might've noticed that those descriptions are basically identical to the JSDocs for the targeted fields.

SchoofsKelvin commented 2 years ago

@SchoofsKelvin Would you like me to work on the other descriptions?

@grtcdr You can do that if you want and create a new PR. I got a few other small issues I need to push out soon, so I'll include your changes in a new extension version soon.

grtcdr commented 2 years ago

You might've noticed that those descriptions are basically identical to the JSDocs for the targeted fields.

I haven't, actually. I've never used or read through JSDoc's docs before.

You can do that if you want and create a new PR.

Great, I'll get to it :)

SchoofsKelvin commented 2 years ago

I've never used or read through JSDoc's docs before.

I meant the JSDoc comments on this file and this file. They're actually the same file present in both projects (the extension itself and the frontend used for the Settings UI), seemed a bit over the top to create a whole common/shared module for just that bit of code. The settings fields are almost perfectly 1-on-1 mapped upon the fields of that config. I mostly copied the descriptions from the FileSystemConfig fields straight to the Settings UI fields.

grtcdr commented 2 years ago

Oh, okay, I get it now.

seemed a bit over the top to create a whole common/shared module for just that bit of code.

But that would just make it a maintenance burden for you if you were to make any modification, right?

SchoofsKelvin commented 2 years ago

But that would just make it a maintenance burden for you if you were to make any modification, right?

Right now, whenever I modify something in one of the files, I should update the other file too. Small burden, and in the end still less work than setting up a whole new module, setting up imports/exports/dependencies, plugging it into the build pipelines, ... which would also increase the build complexity/duration.

And besides, even if both files deviate, not a big issue. As long as each file contains the things that are needed in its project and don't have conflicts, it's not even an issue if I forget.

If I at some point end up with a lot more shared code (which might happen once I add proper Settings UI support for something complex like feature/forwarding), I can still decide to create a common module.

grtcdr commented 2 years ago

Oh okay, thanks for the thorough explanation :)