search

mechatroner / vscode_rainbow_csv

🌈Rainbow CSV - VS Code extension: Highlight CSV and TSV files in different rainbow colors to make them more readable
MIT License
426 stars 51 forks source link

Add SetHeaderLine command #71

Closed WetDesertRock closed 4 years ago

WetDesertRock commented 4 years ago

This adds a SetHeaderLine that will use the currently selected line as a header. This allows the user to easily work around issues posed in #13 and #14. This is also added to the editor context menu to allow users to right click.

WetDesertRock commented 4 years ago

Hi @mechatroner I'd actually disagree with the assessment that we would want one command to encapsulate all possible behavior. At work I use this plugin all of the time on many different CSV files throughout the day. There are frequently are lines in the file before the header or data that confuse this plugin. Having a context menu item to set the current header line would be a large improvement over having to jump into a dialog box just to skip through it. Additionally I don't believe having one command to set the selected line as the header, and one to edit the header manually would be very confusing for the end user. Perhaps I should change the command's text to be "Use selected line as header" to make the behavior more clear.

That being said I do see what you say about the SetVirtualHeader implementation being confusing and agree with your proposed changes. If there are two commands perhaps the SetVirtualHeader should still by default populate with the current header. So if you wanted a different line and then tweak it you could right click, set current line as header, then set virtual header.

After reading your comment I realized there is a bug with my implementation that will prevent it from working on non-csv files properly. I believe I can fix this and the weird "SetVirtualHeader always being comma separated" issue by updating extension.js:264 to use "delim" instead of hardcoding the comma To do this the SetVirtualHeader function would need some minor tweaking in order to still function well.

mechatroner commented 4 years ago

I see, your main objection is that having to skip through the dialog could be annoying, I think this a reasonable objection. OK let's have two commands then. Just fix the bug, please, and I will merge your pull request.

If you want you can also refactor the code to use JSON.stringify(header_array) and save it to the global state but under a different key.

WetDesertRock commented 4 years ago

I somewhat started writing code and forgot half of what you said. I switched over to using JSON.stringify but used the same key. I included code in get_header for compatibility. Theoretically users shouldn't notice a difference.

mechatroner commented 4 years ago

I think this is great! Thank you very much for your contribution, @WetDesertRock ! BTW since we have 2-header related commands, I am thinking about maybe adding a third one: SetVirtualColumnName - which will allow setting the column name just for the column under the cursor.

And there are enough features to make a new release now, so I will start working on it within a week or two, so the next version of Rainbow CSV should be available within a month. I will post an update when it is ready.

mechatroner commented 3 years ago

@WetDesertRock I just published version 1.8.0 which includes the new "SetHeaderLine" command