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

Jump to bottom

Add LspDiag nextWrap and prevWrap command args #482

Merged
merged 3 commits into from
May 8, 2024
Merged

Add LspDiag nextWrap and prevWrap command args #482

merged 3 commits into from
May 8, 2024

Conversation

nickspoons
Copy link
Contributor

Add new 'nextWrap' and 'prevWrap' arguments for the :LspDiag command, allowing wrapping around to the first/last diagnostics instead of displaying the "No more diagnostics found" message.

Closes #413

@nickspoons nickspoons mentioned this pull request Mar 18, 2024
Closed
@yegappan
Copy link
Owner

yegappan commented Apr 4, 2024

Thanks for the patch. Can you fix the following test failure?

command line..script /home/runner/work/lsp/lsp/test/runner.vim[48]..function <SNR>1_LspRunTests[20]..Test_LspDiagsSubcmd
line 4: Expected 'LspDiag first current here highlight last next prev show' but got 'LspDiag first current here highlight last next nextWrap prev prevWrap show'
Test_LspDiagsSubcmd(): FAIL

@nickspoons
Copy link
Contributor Author

Hi @yegappan, I've fixed the test by adding the missing args to the assertion.

yegappan
yegappan requested changes Apr 5, 2024
View reviewed changes
test/clangd_tests.vim
@@ -1625,7 +1625,7 @@ def g:Test_LspDiagsSubcmd()
new XLspDiagsSubCmd.raku

feedkeys(":LspDiag \<C-A>\<CR>", 'xt')
assert_equal('LspDiag first current here highlight last next prev show', @:)
assert_equal('LspDiag first current here highlight last next nextWrap prev prevWrap show', @:)
feedkeys(":LspDiag highlight \<C-A>\<CR>", 'xt')
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add new tests for the nextWrap and prevWrap functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added some commands and assertions to the existing Test_LspDiag_Multi() test, is that OK or do you want separate test functions?

@nickspoons nickspoons requested a review from yegappan May 7, 2024 20:42
@yegappan yegappan merged commit 07a536f into yegappan:main May 8, 2024
@nickspoons nickspoons deleted the wrap-diagnostics branch May 8, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yegappan yegappan Awaiting requested review from yegappan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LspNextDiagnostic at end of diagnostics
2 participants
@nickspoons @yegappan