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

Remove unused showNotifications setting #233

Closed
wants to merge 1 commit into from
Closed

Remove unused showNotifications setting #233

wants to merge 1 commit into from

Conversation

yaegassy
Copy link
Contributor

@yaegassy yaegassy commented Jun 19, 2023

Summary

It appears that the setting for showNotifications has been removed in the latest version of ruff-lsp. Therefore, we have also removed it on the ruff-vscode side.

Test Plan

The following command has been executed/completed.

  • just fmt
  • just check
  • just test

@yaegassy yaegassy changed the title refactor!: remove showNotifications setting Remove unused showNotifications setting Jun 19, 2023
@charliermarsh
Copy link
Member

It actually is used! But it's set as an environment variable by the VS Code extension rather than exposed as an LSP setting. I'm not 100% sure why, but it's how the VS Code extension template is setup. The relevant code in the LSP:

def log_error(message: str) -> None:
    LSP_SERVER.show_message_log(message, MessageType.Error)
    if os.getenv("LS_SHOW_NOTIFICATION", "off") in ["onError", "onWarning", "always"]:
        LSP_SERVER.show_message(message, MessageType.Error)


def log_warning(message: str) -> None:
    LSP_SERVER.show_message_log(message, MessageType.Warning)
    if os.getenv("LS_SHOW_NOTIFICATION", "off") in ["onWarning", "always"]:
        LSP_SERVER.show_message(message, MessageType.Warning)


def log_always(message: str) -> None:
    LSP_SERVER.show_message_log(message, MessageType.Info)
    if os.getenv("LS_SHOW_NOTIFICATION", "off") in ["always"]:
        LSP_SERVER.show_message(message, MessageType.Info)

@yaegassy
Copy link
Contributor Author

Ah, I see.

@yaegassy yaegassy deleted the refactor/remove-show-notifications-setting branch June 19, 2023 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants