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.

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

ci: remove obsoleted if/when-let and shadowed dynamic variable and fix some byte-compile warnings #4612

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

kiennq
Copy link
Member

@kiennq kiennq commented Nov 13, 2024

This fixes:

  • obsoleted if/when-let form
  • shadowed dynamic variables in lsp-protocol (standard-output to be exact)
  • And some byte-compile warnings

There're still quite a few byte-compile warnings left, but I would like to invite anyone who has spare cycles to chimp in to fix them together

@jcs090218
Copy link
Member

Do you know why if-let and when-let is deprecated? I hope they don't since I have used those macros in many places. 😅

@kiennq
Copy link
Member Author

kiennq commented Nov 14, 2024

Do you know why if-let and when-let is deprecated? I hope they don't since I have used those macros in many places. 😅

They switched to if/when-let* form

+++
** 'if-let' and 'when-let' are now obsolete.
Use 'if-let*', 'when-let*' and 'and-let*' instead.

This effectively obsoletes the old '(if-let (SYMBOL SOMETHING) ...)'
single binding syntax, which we'd kept only for backwards compatibility.

Copy link
Member

@jcs090218 jcs090218 left a comment

Choose a reason for hiding this comment

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

It's an improvement, so I'll approve it for the time being. Thanks for taking care of this! :)

@kiennq kiennq enabled auto-merge (squash) November 19, 2024 08:24
@kiennq kiennq merged commit 620bbd7 into emacs-lsp:master Nov 19, 2024
11 of 13 checks passed
@jcs090218
Copy link
Member

It seems like this PR causes a lot of compile warnings. 🤔

In lsp-icons-all-the-icons-icon:
lsp-icons.el:87:2: Warning: docstring wider than 80 characters

Compiling file c:/Users/JenChieh/AppData/Roaming/.emacs.d/elpa/lsp-mode-20241119.828/lsp-ido.el at Wed Nov 20 14:06:45 2024
lsp-ido.el:78:2: Warning: docstring wider than 80 characters

...

In lsp-make-serenata-did-progress-indexing:
lsp-php.el:383:2: Warning: docstring wider than 80 characters

Compiling file c:/Users/JenChieh/AppData/Roaming/.emacs.d/elpa/lsp-mode-20241119.828/lsp-pls.el at Wed Nov 20 14:06:47 2024

Compiling file c:/Users/JenChieh/AppData/Roaming/.emacs.d/elpa/lsp-mode-20241119.828/lsp-prolog.el at Wed Nov 20 14:06:47 2024

Compiling file c:/Users/JenChieh/AppData/Roaming/.emacs.d/elpa/lsp-mode-20241119.828/lsp-protocol.el at Wed Nov 20 14:06:47 2024

In lsp-make-pwsh-script-region:
lsp-protocol.el:413:2: Warning: docstring wider than 80 characters

In lsp-make-omnisharp-ms-build-project:
lsp-protocol.el:415:2: Warning: docstring wider than 80 characters

In lsp-make-omnisharp-run-tests-in-class-request:
lsp-protocol.el:415:2: Warning: docstring wider than 80 characters

In lsp-make-omnisharp-dot-net-test-result:
lsp-protocol.el:415:2: Warning: docstring wider than 80 characters

In lsp-make-omnisharp-metadata-request:
lsp-protocol.el:415:2: Warning: docstring wider than 80 characters

In lsp-make-code-action-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-completion-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-completion-item:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-completion-item-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-completion-options:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-diagnostic:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-document-symbol:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-document-symbol-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-formatting-options:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-server-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-signature-help-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-signature-help-context:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-synchronization-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-text-document-client-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-text-document-sync-options:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-type-hierarchy-item:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-workspace-client-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-workspace-edit-capabilities:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-workspace-file-operations:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-code-action:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-document-on-type-formatting-registration-options:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-initialize-params:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

In lsp-make-location-link:
lsp-protocol.el:603:2: Warning: docstring wider than 80 characters

&allow-other-keys)
(ignore ,@(-map (-lambda ((key))
(intern (substring (symbol-name key) 1))) params))
Copy link
Member

Choose a reason for hiding this comment

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

FYI, I have reverted L116 and L117 in 73ad089 to prevent more compile warnings.

Copy link
Member Author

Choose a reason for hiding this comment

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

This change so that we have the correct doc string for those function. Reverting them will make the docstring turn out to be wrong :(

Can we disable the docstring length warning instead? That would be hard to keep, especially with auto generated function from lsp-interface macro?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I got it. I think there is a variable to configure to turn off the 80-character limit. 🤔

Let me see what I can do about this! 👌

Copy link
Member

Choose a reason for hiding this comment

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

Probably not the best solution, but I've raised the 80-character limit to 1000. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants