Make panels consistent

[predragnikolic]

This is a draft that aims to make the diagnostics, references and rename panel look the same.

Diagnostics before:

Diagnostics after:

References before:

References after:

Rename before:

Rename after:

Questions

1. Should we have a \n after each as a separator between filenames?

plugin/symbols.py:
	   1:28   from .core.protocol import Request, Range
plugin/core/protocol.py:
	  98:7    class Request:

or

plugin/symbols.py:
	   1:28   from .core.protocol import Request, Range

plugin/core/protocol.py:
	  98:7    class Request:

2. I think I can reuse the same syntax file for references to both references and rename panels? Should I delete the Rename.sublime-syntax file? Please give me a suggestion on what to do here. :)

3. The line_regex now must contain a space in front.
   I put a \t in front of any line diagnostics, is that ok change?

[rwols]

Looks like a good opportunity to refactor the "rename" and "references" panel to a common builder class. We're going to need such a class anyway, as we'll also be needing to present the "callees" and "callers" from the new set of call hierarchy requests of 3.16: #1429

[predragnikolic]

a common builder class

Something like this?

class Panel:
    def __init__(self, window: sublime.Window, name: str, unlisted=False):
        self.window = window
        self.panel = self.window.create_output_panel(name, unlisted)
        self.name = "output.{}".format(name)

    def is_open(self):
        return self.window.active_panel() == self.name

    def open(self):
        self.window.run_command("show_panel", {"panel": self.name})

    def hide(self):
        self.window.run_command("hide_panel", {"panel": self.name})

    def update(self, content: str):
        self.panel.run_command("lsp_update_panel", {"characters": content})

    def destroy(self):
        self.window.destroy_output_panel(self.name)

Not sure what you have envisioned :)

[rwols]

Not sure what you have envisioned :)

A builder class... to abstract away all of the boring indent logic and formatting logic. So something like this:

class PanelBuilder:

    def preamble(self, text: str) -> None:
        """Set the preamble text"""
        ...

    def filename(self, filename: str) -> None:
        """Render a filename to the panel"""
        ...

    def diagnostic(self, diagnostic: Diagnostic) -> None:
        """Render a diagnostic to the panel"""
        ...

    def location(self, range: Dict[str, Any], identifier: Optional[str] = None) -> None:
        """Render a reference/rename location"""
        ...

    def __str__(self) -> str:
        """Flush the content as a string"""
        ...

does that make sense?

[predragnikolic]

@rwols I know I haven't implemented your suggestion yet. (for the PanelBuilder)
but I wanted to build a Panel class that abstracts away the interactions with the panel api.

My question is, should I revert the 2a962e0 ?
If the abstraction is too much, or not necessary at the moment I can revert it with no problem :)

[predragnikolic]

I reverted the changes 2a962e0
Can PanelBuilder also be implemented in a different PR?
I just wanted the panels to be consistent :)

[rwols]

1

Should we have a \n after each as a separator between filenames?

I don't see the need for it to be honest.

2

Should I delete the Rename.sublime-syntax file?

Sounds good.

3

The line_regex now must contain a space in front. I put a \t in front of any line diagnostics, is that ok change?

I don't understand why this is needed? Why doesn't SublimeLinter use a tab? http://www.sublimelinter.com/en/stable/

4

Please don't use string concatenation in windows.py

[predragnikolic]

4 - is done.
3 - is done.
2 - is done.
1 - Although a \n is not necessary I would argue that a new line gives a visual separation of the results which help readability. Because of that I would keep the \n between filenames. :)