6 years ago
By "find a more consistent name" you mean that you want to rename the RequestView and RequestsView constructors to be... better? I am not sure what these are "requesting". The doc comments say "Adds a row to the CLI output display" and "A wrapper for a set of rows|command outputs". The doc comments describe to me what they do, but the names don't. I suggest: CommandOutputView and CommandOutputListView. Thoughts?
*like* In the Bespin days, Request was a sensible name, but refactoring broke the name. RequestView(s) came from those days. I was thinking of using ReportView and ReportViews; Mihai complained about the name Report, but I didn't much like his suggestions. But I like your suggestions better. Do you want to take this? It's not just request_view.js that will need changing - there's also canon which has a Report and ReportList which will need changing to CommandOutput and there's a globalReportList which needs to change to commandOutputList (I think - it's not really global, it's just per instance of GCLI) And that affects HUDService.jsm, so you'll need to be able to build FF with the GCLI patches ...
Here is what I am thinking right now: RequestView -> CommandOutputView RequestsView -> CommandOutputListView and then I am not as sure with this last piece: ReportList -> CommandOutput[Controller|Manager] It seems to me we are in agreement with the first two, do you have an opinion on what ReportList should be changed to?
Was thinking CommandOutputList for symmetry with CommandOutputListView?
The thing is, it isn't really a list. Yes, it has a list in the implementation, but none of its public operations involve listy things. Its just a wrapper around an event and manages/controls access to those events. So that's why I like "CommandOutputManager". I am going to just go ahead and continue so that I am being productive; we can always come back and change it later.
I buy the CommandOutputManager argument. Done. Thanks.
Pull request on GitHub: https://github.com/joewalker/gcli/pull/8
Marking verified because there is no UI proof that the bug is fixed. The proof is in the code.