Feature/handle disabled code actions #2402

nbfalcon · 2020-12-11T14:39:14Z

Handle disabled code actions (not shown in lsp-execute-code-action unless C-u prefix specified)
Properly propertize preferred code actions
Add lsp-auto-fix, which executes one of the preferred code actions
Add lsp-execute-code-action-by-type, which executes a code action by given base type (e.g. "refactor", ...)

I have tested this a bit with jdtls, and this PR seems to not introduce a regression. However, I didn't have the chance to test the new disabled and preferred code action handling yet, which is why I am marking as draft.

There will be a corresponding lsp-ui PR soon, too (there is just one minor change I need to make, and to test this).

yyoncho · 2020-12-11T15:22:26Z

lsp-mode.el

+(lsp-defun lsp--code-action-title ((action &as &CodeAction :title :is-preferred? :disabled?))
+  ;; It would be strange for a code action to be both preferred and disabled, so
+  ;; `cond' is probably good enough.
+  (cond (is-preferred?


Ideally we would want the default code action to be preselected in the select dialog. If you don't want to adress it in this PR you may leave a TODO.

I have implemented this. That feature was surprisingly easy to do, requiring only a small change to lsp-completing-read and one to lsp--select-action.

yyoncho · 2020-12-11T15:25:43Z

lsp-mode.el

-                                        #'lsp:code-action-title)
-                              nil t)))))
+  (let ((actions (seq-into actions 'list)))
+    (cond


If you are going to change it, then change it to pcase.

yyoncho · 2020-12-11T15:32:42Z

lsp-mode.el

+  (->> (lsp-get-or-calculate-code-actions command-kind)
+       (-filter (-lambda ((&CodeAction :kind?))
+                  (and kind? (equal command-kind kind?))))
+       lsp--select-enabled-action


If I call explicitly a refactoring and the method is disabled I would like to see the reason for which it is disabled. E. g. if I call "Extract method" I might see an error "You have selected a range that cannot be extracted to a method".

Fixed. There might be a prompt with the reasons listed individually, depending on the number of actions and lsp-auto-execute-action.

yyoncho · 2020-12-11T15:35:59Z

lsp-mode.el

+    (-when-let ((&CodeAction :disabled?) action)
+      (user-error "%s is disabled: %s" (lsp:code-action-title action)
+                  (lsp:code-action-disabled-reason disabled?)))


this seems like should be part of lsp-execute-code-action. E. g. if lsp-ui calls the method with disabled action it should get that error.

Note: this is now an error instead of a user-error (reasoning in the commit message).

yyoncho · 2020-12-11T15:36:49Z

lsp-mode.el

+    (funcall (if current-prefix-arg
+                 #'lsp--select-action-enabled-or-die
+               #'lsp--select-enabled-action)
+             (lsp-code-actions-at-point))))


seems like if you drop lsp--select-enabled-action/lsp--select-action-enabled-or-die and pass an optional parameter to lsp-code-actions-at-point the code will be simplified/shortened a lot.

nbfalcon · 2020-12-11T17:44:27Z

Should I prepend "Review: " to the commit subject if it doesn't make it go over 50 lines?

yyoncho · 2020-12-11T19:03:07Z

Should I prepend "Review: " to the commit subject if it doesn't make it go over 50 lines?

We dont have such a requirement. It will be squashed in the end so it shouldn't matter.

yyoncho

Looks good. I would have approved it if it was not marked as draft.

yyoncho · 2020-12-11T20:37:57Z

lsp-mode.el

+  "Retrieve the code actions for the active region or the current line.
+If ENABLED is specified, only return code actions that are not
+disabled."
+  (--doto (lsp-request "textDocument/codeAction" (lsp--text-document-code-action-params kind))


I guess this section is on the edge of being too trickier.

What I had in mind here was the following: currently there is a single filter: ENABLED. In the future, more filters could be added, and using --doto with cl-callf allows adding them easily (e.g. is-preferred). I could rewrite this in terms of if if you consider that to be a large enough readability gain:

(let ((actions (lsp-request "textDocument/codeAction" (lsp--text-document-code-action-params kind)))) (if enabled (seq-remove #'lsp:code-action-disabled? actions) acitons))

I just realized that the code has a bug: lsp-code-action-disabled? should be lsp:code-action-disabled?.

I guess it could stay as it is, just it is a bit unusual. Once -cond->> lands in dash we can use it here(and few other places).

-cond->> would certainly be awesome, and I remember that I wanted to use it somewhere but couldn't. However, I don't see how this would help here? The point of this extensible function style is to allow filters to be chained: e.g. first filter out all disabled actions, then those that don't match the type base, .... (we don't have that yet, but might in the future).

Something like that (clj)

(let [even true small true] (cond->> (range 20) even (filter even?) small (filter #(< 10 %))))

yyoncho · 2020-12-11T20:39:45Z

lsp-mode.el

+       lsp--select-action
+       lsp-execute-code-action))
+
+(defun lsp-execute-code-action-by-type (kind)


can you elaborate on why we need this method?

If there is some specific code action to be executed, e.g. source.organizeImports, lsp-execute-code-action-by-kind is enough. However, sometimes it can be useful to bring up all quickfixes, or all refactorings: in this case, lsp-execute-code-action-by-kind doesn't work, because it matches the kind exactly. My new function matches a base kind: lsp-execute-code-action-by-type "refactor" would match "refactor.move", "refactor.*", ... (tested with jdtls). This corresponds to the spec's "if the user requests a more specific type of code action, e.g. refactoring".

yyoncho · 2020-12-11T20:42:47Z

lsp-mode.el

-                        lsp--select-action)))
-      (lsp-execute-code-action action)
-    (signal 'lsp-no-code-actions '(command-kind))))
+  (->> (lsp-get-or-calculate-code-actions command-kind)


don't we need to filter the disabled and only in case there is no enabled action use the disabled one(s) error message?

nbfalcon · 2020-12-11T21:46:38Z

Now all that is left is to test this.

nbfalcon · 2020-12-11T22:05:24Z

As a side note, why not directly wait for lsp-response-timeout here? I.e. (accept-process-output nil lsp-response-timeout).

yyoncho · 2020-12-11T22:13:43Z

As a side note, why not directly wait for lsp-response-timeout here? I.e. (accept-process-output nil lsp-response-timeout).

We are not doing accept-process-output on the process that we need to read from and also we need to read multiple messages and potentially from different processes. As a side note, this code should be rewritten to use emacs 26 threads.

leungbk · 2020-12-13T06:09:20Z

As a side note, this code should be rewritten to use emacs 26 threads.

I've heard Emacs' threading implementation isn't thread-safe:

ThreadSanitizer (TSan) quickly shows that Emacs’ threading implementation has many data races, making it completely untrustworthy. Until this is fixed, nobody should use Emacs threads for any purpose, and threads should disabled at compile time.

https://nullprogram.com/blog/2018/05/31/

yyoncho · 2020-12-13T09:28:40Z

As a side note, this code should be rewritten to use emacs 26 threads.

I've heard Emacs' threading implementation isn't thread-safe:

ThreadSanitizer (TSan) quickly shows that Emacs’ threading implementation has many data races, making it completely untrustworthy. Until this is fixed, nobody should use Emacs threads for any purpose, and threads should disabled at compile time.

https://nullprogram.com/blog/2018/05/31/

As per Eli's comments, it is not that bad as described in this blogpost.

nbfalcon · 2021-01-01T13:06:28Z

lsp-mode.el

@@ -4898,9 +4898,9 @@ Shown after the code action in `lsp-execute-code-action',
        (disabled?
         (concat
          (propertize title 'face 'lsp-disabled-code-action-face)
-          " "


Do we need this commit at all? I don't know how far you want to go to support spec-breaking LS.

Sorry, I got lost. Can you elaborate? There are several improvements in this PR so IMHO better to have them in.

Code actions can have a disabled? field, which, as per the spec, must have a reason member. This commit handles the case where language servers send invalid code actions that have a disabled field without a reason (I don't know of an LS that does that, though). This is just being defensive, and I'm unsure whether we need to handle that case (it is a spec violation after all).

Most of the time we don't handle them. Leaving up to you to decide.

I propose to keep it in then, because it can't hurt to have a more lenient lsp implementation.

kiennq · 2021-01-02T00:02:38Z

lsp-mode.el

+                           (-filter (-lambda ((&CodeAction :kind?))
+                                      (and kind? (equal command-kind kind?))))))
+         (enabled-actions (-remove #'lsp:code-action-disabled? all-actions))
+         (action (lsp--select-action (or enabled-actions all-actions))))


Is this ok? In case of all actions are disabled, we show all actions instead of empty list?
Also, shouldn't we need guard of action as the old code, in case of user not select anything.

The idea is to show an error message in case there is no enabled code action, if the user has enabled lsp-auto-execute-action as t (discussion). However, I agree that this might be suboptimal. Do you have an idea for a better UI in this case?

As for guarding, we needn't do that, since the user cannot exit from lsp--select-action without either selecting an action or quitting (causing a non-local exit), due to REQUIRE-MATCH being set as t.

Isn't lsp-auto-execute-action is only for single code action?
The case of lsp-execute-code-action-by-kind is falling into the bellow in the spec, should we just take the first code action and let-bind lsp-auto-execute-action to t so the error will always be shown?

* - If the user has a keybinding that auto applies a code action and only * a disabled code actions are returned, the client should show the user * an error message with `reason` in the editor.

We could let-bind lsp-auto-execute-action to t if there are no enabled ones, however I am unsure if that is the right thing: lsp-execute-code-action-by-kind doesn't auto-execute a single code action if available by default, either. If there is only one disabled code action, a prompt containing only it will show up, and along the code action title the disabled-reason will be shown:

Shouldn't lsp--select-action automatically choose the only one code action available when lsp-auto-execute-action is t?
Is there special case for disable code action that make the lsp--select-action behaves otherwise?

lsp-mode.el

kiennq · 2021-01-02T00:06:56Z

lsp-mode.el

+... (see `lsp-preferred-code-action-face').")
+
+(lsp-defun lsp--code-action-title ((action &as &CodeAction :title :is-preferred? :disabled?))
+  ;; It would be strange for a code action to be both preferred and disabled, so


Fixed. In what cases do we need docstrings? In all new code, or just in certain functions?

I think we should have docstring in all new function

I'll keep that in mind.

kiennq · 2021-01-02T00:38:13Z

lsp-mode.el

+                                 (-compose (lsp--create-unique-string-fn)
+                                           #'lsp--code-action-title)
+                                 nil t nil nil
+                                 (or (-find-index #'lsp:code-action-is-preferred? actions)


These should be -find instead of -find-index, the index passing is only working in ivy-mode

I have implemented that for lsp--completing-read.

Ah, sorry, I missed that.
Also, why don't just pass using -find instead of -find-index?
It seems that you're taking a round trip of finding index and taking the list element by index here.

If the COLLECTION is a list of strings that is to be transformed, there would be an ambiguitiy: should DEF be used to look up the corresponding string in COLLECTION, or be the DEF argument to completing read?

Wouldn't that ambiguity be resolved if we document it? For example, DEF should be the corresponding string in COLLECTION (since it's what's returned). That would also be aligned with original completing-read which is what wrapped in lsp--completing-read.

nbfalcon · 2021-01-02T12:55:25Z

This PR is done. I will not be adding any new commits/features except those needed to address review comments.

kiennq · 2021-01-02T13:33:09Z

lsp-mode.el

+  "Face used to highlight disabled code actions.
+See `lsp-preferred-code-action-face' for details.")
+
+(defface lsp-disabled-code-action-reason-face '((t :inherit lsp-details-face))


This is not used?

Fixed. I wanted to add a help-echo to disabled code actions at some point, but then realized that it wouldn't work with completing-read.

Nevermind, this is a regression from the commit to support both preferred and disabled code actions at the same time. It was supposed to show the reason. I'll need to add it back.

kiennq · 2021-01-02T19:31:37Z

lsp-mode.el

+       lsp--select-action
+       lsp-execute-code-action))
+
+(defun lsp-quickfix ()


There's lsp-make-interactive-code-action macro which use lsp-execute-code-action-by-kind, do you want to provide similar one for lsp-execute-code-action-by-type?

michaelpj · 2021-02-28T14:14:01Z

lsp-mode.el

-(defun lsp-code-actions-at-point (&optional kind)
-  "Retrieve the code actions for the active region or the current line."
-  (lsp-request "textDocument/codeAction" (lsp--text-document-code-action-params kind)))
+(defun lsp-code-actions-at-point (&optional kind enabled)


While we're here... could we make this a list of kinds? The protocol supports that.

michaelpj · 2021-02-28T14:15:36Z

lsp-mode.el

+disabled."
+  (--doto (lsp-request "textDocument/codeAction" (lsp--text-document-code-action-params kind))
+    (when enabled
+      (cl-callf2 seq-remove #'lsp:code-action-disabled? it))))

 (defun lsp-execute-code-action-by-kind (command-kind)
  "Execute code action by name."


Suggested change

"Execute code action by name."

"Execute code action by kind."

michaelpj · 2021-02-28T14:16:59Z

lsp-mode.el

+         (action (lsp--select-action (or enabled-actions all-actions))))
+    (lsp-execute-code-action action)))
+
+(defun lsp-execute-code-action-by-type (kind &optional enabled)


The difference between this and lsp-execute-code-action-by-kind isn't clear to me (or from the docs).

lsp-execute-code-action-by-type matches base-types, i.e. lsp-execute-code-action-by-type "source" would match "source.organizeImports", while -by-kind matches exactly, i.e. the same example would yield nothing, and it would have to be (lsp-execute-code-action-by-kind "source.organizeImports").

I'm not sure this is a distinction the spec really knows about - certainly "base type" doesn't appear. They do talk about "base kinds", but I'm not sure they're meaningfully distinct from "kinds", it just indicates that they should be hierarchical parents of a set of kinds.

What about just having an exact-match argument to lsp-execute-code-action-by-kind? Or maybe we shouldn't even provide exact matching. The intention for spec methods that allow you to do kind-based filtering is to include kinds "under" the selected kinds (see e.g. microsoft/language-server-protocol#970 (comment)). Do we have a usecase for exact matching?

michaelpj · 2021-02-28T14:19:33Z

lsp-mode.el

@@ -5224,10 +5293,19 @@ It will show up only if current point has signature help."
      (funcall action-handler action)
    (lsp--send-execute-command command arguments?)))

-(lsp-defun lsp-execute-code-action ((action &as &CodeAction :command? :edit?))
+(lsp-defun lsp-execute-code-action ((action &as &CodeAction :command? :edit? :disabled?))


Should this have an optional kinds argument? Generally, it seems like almost all the functions that operate on code actions could take an optional kinds argument, perhaps that would let us get away with fewer functions.

Not in this case. lsp-execute-code-action should execute whatever action it is given. It is only additionally interactive.

As of version 3.16, CodeActions may have an optional disabled? property, which should cause them to be ignored in certain cases (see the spec for details). It is a structure, containing a reason, so model with a new interface, `CodeActionDisabled`.

In that function, filter the actions to only include non-disabled ones. This way, `lsp-no-code-actions' is `signal'ed even when there are code actions that are all `:disabled?'. This is consistent with the spec guidelines: "disabled code actions are not shown in automatic light bulb code action menus". Given that we now support it at least somewhere, set the "disabledSupport" capability.

`lsp--select-action': mark preferred code actions using `lsp-preferred-code-action-face'.

With a C-u prefix, `lsp-execute-code-action' now also shows disabled code actions. These are propertized with `lsp-disabled-code-action-face', and have the reason appended to them. To implement this, make `lsp--select-action' no longer filter the actions, and to use the new `lsp--code-action-title' to render their tiles. The new function checks whether the code action is preferred or disabled and returns a proper title. A new function, `lsp--select-enabled-action' is the same as the above but filters out disabled candidates. Use it in `lsp-execute-code-action-by-kind', because it is usually used to execute specific, global actions where it wouldn't make sense to have a prompt if they are unavailable.

The `if-let' around the call to `lsp--select-enabled-action' is redundant, because that function can't ever return nil: if there are no code actions, it signals `lsp-no-code-actions' (as the old code did), and for the user to select nothing from `completing-read', they would have to `quit', aborting execution. This will allow that check to be ommitted from similar functions while retaining code consistency.

- `lsp-execute-code-action-by-type': execute a given code action by its base type (e.g. refactor) - `lsp-auto-fix': apply the first preferred code action

Using `pcase' makes the code more readable.

Handle code actions being disabled in `lsp-execute-code-action'. This way, other callers need not do the check + `error' themselves, reducing code duplication and future bugs. Since that function can be called by others and it isn't necessarily the user's fault that an incorrect action was passed, use `error'.

Extend `lsp-completing-read' to allow DEF to be a number the index into collection that is to be pre-selected. In `lsp--select-action', pass the index of the first `lsp:code-action-is-preferred' item in the given list as that argument, pre-selecting it. Using an index into the collection is needed in `lsp-completing-read', because the given TRANSFORMER-FN is often not side-effect-free (as is the case in `lsp--select-action', due to `lsp--create-unique-string-fn'), so transforming whatever default is to be used first and passing the resulting string as DEF is not possible.

Drop `lsp--select-enabled-action' and add an optional ENABLED parameter to `lsp-code-actions-at-point' instead. This simplifies the code and would allow passing this parameter as a language server hint in the future.

`lsp-code-action-disabled?' checks whether the argument is an `&CodeActionDisabled' struct, not whether the given code action has the :disabled? property set to true. Use `lsp:code-action-disabled?' instead.

If there are enabled code actions, choose them. Otherwise, select from the disabled ones, showing the error message(s).

The optional argument ENABLED, if given, filters out all disabled code actions. This way, code action commands can be implemented that sacrifice discoverability for efficiency.

Technically, the LSP protocol doesn't allow disabled code actions to not specify a reason. However, this might still be the case for language servers breaking the spec slightly, so defensively handle the case of it being omitted or null.

If there are no preferred code actions, pre-select the first one that isn't disabled.

With `lsp-execute-code-action-by-type' added, it makes sense to expose some common types as commands so that users don't have to define their own functions for that.

`lsp--code-action-title': handle both preferred and disabled code actions, because the LSP protocol does not forbid that case. Such a code action might be a refactoring that addresses the issue but can't be executed because there isn't enough RAM, for example.

Similar to `lsp-make-interactive-code-action', `lsp-make-interactive-code-action-by-type' defines a function to execute a given base type (using `lsp-execute-code-action-by-type'). Use it to define `lsp-quickfix' and `lsp-refactor'.

michaelpj · 2021-06-25T14:39:00Z

It would be nice to get this in. The helpers for executing code actions by kind would be quite nice for clients.

nbfalcon marked this pull request as draft December 11, 2020 14:39

yyoncho reviewed Dec 11, 2020

View reviewed changes

nbfalcon commented Jan 1, 2021

View reviewed changes

nbfalcon mentioned this pull request Jan 1, 2021

Feature/lsp UI sideline disabled preferred actions emacs-lsp/lsp-ui#545

Draft

nbfalcon marked this pull request as ready for review January 1, 2021 13:35

kiennq reviewed Jan 2, 2021

View reviewed changes

lsp-mode.el Show resolved Hide resolved

kiennq reviewed Jan 2, 2021

View reviewed changes

lsp-mode.el Show resolved Hide resolved

kiennq reviewed Jan 2, 2021

View reviewed changes

nbfalcon force-pushed the feature/handle-disabled-code-actions branch from 775c11a to 7ab2f86 Compare January 2, 2021 14:22

kiennq reviewed Jan 2, 2021

View reviewed changes

nbfalcon mentioned this pull request Feb 28, 2021

Add functions to operate on code actions based on their kind #2656

Open

michaelpj reviewed Feb 28, 2021

View reviewed changes

yyoncho approved these changes Feb 28, 2021

View reviewed changes

nbfalcon added 2 commits March 16, 2021 20:55

nbfalcon added 17 commits March 16, 2021 20:55

lsp--select-code-action: highlight preferred

4865379

`lsp--select-action': mark preferred code actions using `lsp-preferred-code-action-face'.

Add new code action commands

0f48af3

- `lsp-execute-code-action-by-type': execute a given code action by its base type (e.g. refactor) - `lsp-auto-fix': apply the first preferred code action

Review: lsp--select-action': use pcase'

e6342b4

Using `pcase' makes the code more readable.

Refactor: remove `lsp--select-enabled-action'

cb1897a

Drop `lsp--select-enabled-action' and add an optional ENABLED parameter to `lsp-code-actions-at-point' instead. This simplifies the code and would allow passing this parameter as a language server hint in the future.

`lsp-code-actions-at-point': fix disabled? check

801d014

`lsp-code-action-disabled?' checks whether the argument is an `&CodeActionDisabled' struct, not whether the given code action has the :disabled? property set to true. Use `lsp:code-action-disabled?' instead.

`lsp-execute-code-action-by-kind': choose enabled

ace8758

If there are enabled code actions, choose them. Otherwise, select from the disabled ones, showing the error message(s).

`lsp-execute-code-action-by-type': ENABLED filter

74a828e

The optional argument ENABLED, if given, filters out all disabled code actions. This way, code action commands can be implemented that sacrifice discoverability for efficiency.

`lsp--code-action-title': support omitted reason

abedf27

Technically, the LSP protocol doesn't allow disabled code actions to not specify a reason. However, this might still be the case for language servers breaking the spec slightly, so defensively handle the case of it being omitted or null.

`lsp--select-action': pre-select non-disabled

8367f49

If there are no preferred code actions, pre-select the first one that isn't disabled.

Add lsp-quickfix' and lsp-refactor' commands

7f65d1c

With `lsp-execute-code-action-by-type' added, it makes sense to expose some common types as commands so that users don't have to define their own functions for that.

`lsp--code-action-title': add docstring

7b55270

Add a macro to expose base kinds as commands

05d5271

Similar to `lsp-make-interactive-code-action', `lsp-make-interactive-code-action-by-type' defines a function to execute a given base type (using `lsp-execute-code-action-by-type'). Use it to define `lsp-quickfix' and `lsp-refactor'.

nbfalcon force-pushed the feature/handle-disabled-code-actions branch from 7ab2f86 to 05d5271 Compare March 16, 2021 20:11

michaelpj mentioned this pull request Jun 25, 2021

added: lsp-haskell-execute-code-action-add-signature emacs-lsp/lsp-haskell#129

Open

yyoncho closed this Feb 23, 2023

	"Execute code action by name."
	"Execute code action by kind."

Uh oh!

Feature/handle disabled code actions #2402

Feature/handle disabled code actions #2402

Uh oh!

Conversation

nbfalcon commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbfalcon Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbfalcon commented Dec 11, 2020

Uh oh!

yyoncho commented Dec 11, 2020

Uh oh!

yyoncho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbfalcon Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbfalcon commented Dec 11, 2020

Uh oh!

nbfalcon commented Dec 11, 2020

Uh oh!

yyoncho commented Dec 11, 2020

Uh oh!

leungbk commented Dec 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yyoncho commented Dec 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbfalcon commented Dec 11, 2020 •

edited

Loading

nbfalcon Dec 11, 2020 •

edited

Loading

nbfalcon Dec 11, 2020 •

edited

Loading

leungbk commented Dec 13, 2020 •

edited

Loading