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

Add missing configuration options for the Ada Language Server. #4567

Merged
merged 2 commits into from
Dec 14, 2024

Conversation

brownts
Copy link
Contributor

@brownts brownts commented Sep 30, 2024

Additionally, prefer "initialization-options" over "initialized-fn" as the Ada Language Server prefers to know the project file as part of initialization, and will have to create a default one if it's not specified as part of these options. If a project file is not specified as part of the initialization options, the server might also display a diagnostic about no project being specified until a workspace/didChangeConfiguration is received specifying the project file.

Add support for Ada run-time library folders via the new user option lsp-ada-library-folders, defaulting to detecting the run-time directory with it's unique "adainclude" folder.

@github-actions github-actions bot added the client One or more of lsp-mode language clients label Sep 30, 2024
@brownts brownts force-pushed the bugfix/ada-settings branch from 7c4b851 to 5c34b1e Compare September 30, 2024 22:56
:type 'string
:group 'lsp-ada
:package-version '(lsp-mode . "6.2")
:lsp-path "ada.defaultCharset")
;;;###autoload(put 'lsp-ada-option-charset 'safe-local-variable 'stringp)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to autoload all these custom variables? 🤔

Copy link
Contributor Author

@brownts brownts Oct 1, 2024

Choose a reason for hiding this comment

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

The reason I added them is that any of those could potentially be specified in a .dir-locals.el. I find it annoying to have to manually approve the "potentially unsafe" settings in my .dir-locals.el. If you think this is too much, I can cut these back to the most frequently used options. I could of course add safety settings in my init.el, but since other people use this mode as well, it would require others to do the same in their init.el, and I was going for ease of use.

Copy link
Member

Choose a reason for hiding this comment

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

Let's cut back for now since it's unusual for only this file to do that. 🤔 We can open a new issue, discuss this, and see what others think.

Additionally, prefer "initialization-options" over "initialized-fn" as
the Ada Language Server prefers to know the project file as part of
initialization, and will have to create a default one if it's not
specified as part of these options.  If a project file is not
specified as part of the initialization options, the server might also
display a diagnostic about no project being specified until a
workspace/didChangeConfiguration is received specifying the project
file.

Due to a current issue with the Ada Language Server, "initialized-fn"
is kept to work around an issue where the project specified in
"initialization-options" might not be honored.

Add support for Ada run-time library folders via the new user option
`lsp-ada-library-folders`, defaulting to detecting the run-time
directory with it's unique "adainclude" folder.
The Ada Language Server archive name, archive type and executable
location within the archive all changed recently.  This caused the
server install/upgrade to no longer function.  This change rectifies
those issues.
@brownts brownts force-pushed the bugfix/ada-settings branch from 5c34b1e to 40dea56 Compare December 8, 2024 22:13
@brownts
Copy link
Contributor Author

brownts commented Dec 8, 2024

I've updated the commit to remove all of the additional autoloads. I've also added a second commit which fixes the Ada Language Server installation, as the name, archive and path within the archive all changed recently.

@brownts
Copy link
Contributor Author

brownts commented Dec 14, 2024

@jcs090218, did you have any further concerns?

@jcs090218 jcs090218 merged commit 7d8f232 into emacs-lsp:master Dec 14, 2024
10 of 13 checks passed
@jcs090218
Copy link
Member

Merged! Thank you for taking care of this! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client One or more of lsp-mode language clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants