Open jeffreymcclain opened 4 years ago
hey @jeffreymcclain - Apologies on the delay here, it looks like this got buried! Getting the parameter aliases added in with the next release, will also see what I can do about falling back in the event of an error to gather more intelligent context around the root cause of the error, e.g. bad Sheet name, etc. I do like the idea of having a more descriptive, generic error as a last resort as well!
I started playing around with this. First things first, I played around with Parameter Sets, so that either SheetName
, Range
, or both are required. I also made a default ParameterSet that includes either, so that running it without either gives you an error message we can control (currently Neither SheetName nor Range was specified, one or both must be specified. SheetName refers to the "sheets" or tabs at the bottom of a Spreadsheet, not the title of the Spreadsheet.
). I don't like creating a parameter set that's guaranteed to fail, but the alternative is a very unhelpful error of the form "Parameter set cannot be resolved" which doesn't give any information, and which we have no control over.
That would (I think) cover the very initial case where the command was called with just the Spreadsheet ID, because it would tell you to include the Sheet name.
Next up is being a little smarter with that range error.
OK, so I ended up going back on all the parameter sets, and ended up with this: https://github.com/SCRT-HQ/PSGSuite/compare/4bfefc67cd930ad6b01315e057959139eb32946f...9caf2fc9dd5583625e921a5acd524309513a1c4e
I added some more help text and examples. The function will still let you execute with neither SheetName nor Range, but it will fail with an explicit error that you need to specify one of them, explain what's meant by "Sheet".
It will also try to give some useful information instead of giving the generic Unable to parse range:
Error. If you specified a SheetName that matches the title of the Spreadsheet, it will warn you that you need to pick a sheet name, not the spreadsheet name. Otherwise it will still try and give you some hint that you may not have specified SheetName.
The reason SheetName isn't required is that it's possible to use the Range parameter instead of SheetName, but that's an "advanced" case and SheetName is what you'd want to use, unless you have any understanding of the "guts" of the API.
Currently, entering an incorrect sheet name yields this error message. @FISHMANPET helped troubleshoot my issue, and eventually he determined that it was caused by me entering the workbook name instead of the sheet name.
My thought is the error message could be more descriptive (e.g. "Error: Verify you entered the sheet name, NOT spreadsheet name!"). This message could be displayed dynamically using an API call to determine if the sheet name has indeed been entered incorrectly (though if that is possible, imo it would be better to just make the "SheetName" parameter optional and have the default value be the first sheet name).
Alternately, since this might be a common issue, it could simply be part of the "default" error message if that cmdlet fails with the particular error code from the screenshot (e.g. "Error: Something went wrong, verify that you entered the tab / sheet name and not the spreadsheet / workbook name.")
P.S.: Currently the "SheetName" parameter is "optional" (the cmdlet itself does not have it flagged as mandatory), however afaik the command will always fail if the SheetName parameter is not manually specified, meaning it should probably be mandatory (unless I am missing something).