Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements for DocumentLink in hover popup #2098

Merged
merged 3 commits into from
Oct 25, 2022
Merged

Improvements for DocumentLink in hover popup #2098

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
947c003
Improvements for DocumentLink in hover popup
jwortmann Oct 23, 2022
cba0321
else branch not needed because there was a typo
jwortmann Oct 23, 2022
5807b3c
Another small fix
jwortmann Oct 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 40 additions & 26 deletions plugin/hover.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from .core.sessions import SessionBufferProtocol
from .core.settings import userprefs
from .core.typing import List, Optional, Dict, Tuple, Sequence, Union
from .core.url import parse_uri
from .core.views import diagnostic_severity
from .core.views import first_selection_region
from .core.views import format_code_actions_for_quick_panel
Expand All @@ -38,7 +39,7 @@
from .core.views import unpack_href_location
from .core.views import update_lsp_popup
from .session_view import HOVER_HIGHLIGHT_KEY
import functools
from functools import partial
import html
import sublime

Expand Down Expand Up @@ -123,7 +124,7 @@ def run(
hover_point = temp_point
self._base_dir = wm.get_project_path(self.view.file_name() or "")
self._hover_responses = [] # type: List[Tuple[Hover, Optional[MarkdownLangMap]]]
self._document_link = ('', False, None) # type: Tuple[str, bool, Optional[sublime.Region]]
self._document_links = [] # type: List[DocumentLink]
self._actions_by_config = [] # type: List[CodeActionsByConfigName]
self._diagnostics_by_config = [] # type: Sequence[Tuple[SessionBufferProtocol, Sequence[Diagnostic]]]
# TODO: For code actions it makes more sense to use the whole selection under mouse (if available)
Expand Down Expand Up @@ -157,9 +158,7 @@ def request_symbol_hover_async(self, listener: AbstractViewListener, point: int)
Request("textDocument/hover", document_position, self.view)
))
language_maps.append(session.markdown_language_id_to_st_syntax_map())

continuation = functools.partial(self._on_all_settled, listener, point, language_maps)
Promise.all(hover_promises).then(continuation)
Promise.all(hover_promises).then(partial(self._on_all_settled, listener, point, language_maps))

def _create_hover_request(
self, session: Session, point: int
Expand Down Expand Up @@ -206,8 +205,7 @@ def request_document_link_async(self, listener: AbstractViewListener, point: int
link_promises.append(sv.session.send_request_task(Request.resolveDocumentLink(link, sv.view)).then(
lambda link: self._on_resolved_link(sv.session_buffer, link)))
if link_promises:
continuation = functools.partial(self._on_all_document_links_resolved, listener, point)
Promise.all(link_promises).then(continuation)
Promise.all(link_promises).then(partial(self._on_all_document_links_resolved, listener, point))

def _on_resolved_link(self, session_buffer: SessionBufferProtocol, link: DocumentLink) -> DocumentLink:
session_buffer.update_document_link(link)
Expand All @@ -216,20 +214,7 @@ def _on_resolved_link(self, session_buffer: SessionBufferProtocol, link: Documen
def _on_all_document_links_resolved(
self, listener: AbstractViewListener, point: int, links: List[DocumentLink]
) -> None:
contents = []
link_has_standard_tooltip = True
for link in links:
target = link.get("target")
if not target:
continue
title = link.get("tooltip") or "Follow link"
if title != "Follow link":
link_has_standard_tooltip = False
contents.append('<a href="{}">{}</a>'.format(html.escape(target), html.escape(title)))
if len(contents) > 1:
link_has_standard_tooltip = False
link_range = range_to_region(links[0]["range"], self.view) if links else None
self._document_link = ('<br>'.join(contents) if contents else '', link_has_standard_tooltip, link_range)
self._document_links = links
self.show_hover(listener, point, only_diagnostics=False)

def _handle_code_actions(
Expand All @@ -248,6 +233,26 @@ def symbol_actions_content(self, listener: AbstractViewListener, point: int) ->
actions = [lk.link(point, self.view) for lk in link_kinds if self.provider_exists(listener, lk)]
return " | ".join(actions) if actions else ""

def link_content_and_range(self) -> Tuple[str, Optional[sublime.Region]]:
if len(self._document_links) > 1:
combined_region = range_to_region(self._document_links[0]["range"], self.view)
for link in self._document_links[1:]:
combined_region = combined_region.cover(range_to_region(link["range"], self.view))
if all(link.get("target") for link in self._document_links):
return '<a href="quick-panel:DocumentLink">Follow Link…</a>', combined_region
else:
return "Follow Link…", combined_region
elif len(self._document_links) == 1:
link = self._document_links[0]
target = link.get("target")
label = "Follow Link" if link.get("target", "file:").startswith("file:") else "Open in Browser"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably go with Open in Browser if starts with http and otherwise Follow Link because there could be cases where link would use a custom protocol like virtual: or something and it would be a link to a local content.

Copy link
Member

@rchl rchl Oct 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but I see that _on_navigate handler would then have to be somehow changed to follow that logic...

title = link.get("tooltip")
tooltip = ' title="{}"'.format(html.escape(title)) if title else ""
region = range_to_region(link["range"], self.view)
return '<a href="{}"{}>{}</a>'.format(html.escape(target), tooltip, label) if target else label, region
else:
return "", None

def diagnostics_content(self) -> str:
formatted = []
for sb, diagnostics in self._diagnostics_by_config:
Expand Down Expand Up @@ -283,17 +288,15 @@ def show_hover(self, listener: AbstractViewListener, point: int, only_diagnostic
def _show_hover(self, listener: AbstractViewListener, point: int, only_diagnostics: bool) -> None:
hover_content = self.hover_content()
contents = self.diagnostics_content() + hover_content + code_actions_content(self._actions_by_config)
link_content, link_has_standard_tooltip, link_range = self._document_link
link_content, link_range = self.link_content_and_range()
only_link_content = not bool(contents) and link_range is not None
prefs = userprefs()
if prefs.show_symbol_action_links and contents and not only_diagnostics and hover_content:
symbol_actions_content = self.symbol_actions_content(listener, point)
if link_content and link_has_standard_tooltip:
if link_content:
if symbol_actions_content:
symbol_actions_content += ' | '
symbol_actions_content += link_content
elif link_content:
contents += '<div class="link with-padding">' + link_content + '</div>'
if symbol_actions_content:
contents += '<div class="actions">' + symbol_actions_content + '</div>'
elif link_content:
Expand Down Expand Up @@ -346,13 +349,24 @@ def _on_navigate(self, href: str, point: int) -> None:
placeholder="Code actions")
else:
self.handle_code_action_select(config_name, actions, 0)
elif href == "quick-panel:DocumentLink":
window = self.view.window()
if window:
targets = [link["target"] for link in self._document_links] # pyright: ignore

def on_select(targets: List[str], idx: int) -> None:
if idx > -1:
self._on_navigate(targets[idx], 0)

window.show_quick_panel(
[parse_uri(target)[1] for target in targets], partial(on_select, targets), placeholder="Open Link")
elif is_location_href(href):
session_name, uri, row, col_utf16 = unpack_href_location(href)
session = self.session_by_name(session_name)
if session:
position = {"line": row, "character": col_utf16} # type: Position
r = {"start": position, "end": position} # type: Range
sublime.set_timeout_async(functools.partial(session.open_uri_async, uri, r))
sublime.set_timeout_async(partial(session.open_uri_async, uri, r))
else:
open_in_browser(href)

Expand Down