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

Implement a "compact" flag #66

Merged
merged 2 commits into from
Oct 22, 2021
Merged

Conversation

WhyNotHugo
Copy link
Contributor

This flag removes the header and extra spacing. It's suitable when using
lsp_document_diagnostics, where including the filename and collapse
icon is redundant (since there's only one file anyway).

I'm mostly using Trouble with lsp_document_diagnostics, so I only see errors for the current file. I'd rather keep the error split small and fit as much content as possible, and this helps with that goal.

The default behaviour remains the same, since the default is compact=false.

Let me know if the change seems too invasive. I'd also like to add a mode where only diagnostics for the current line are shown, and compact fits well with that.

@hanspinckaers
Copy link
Contributor

Thanks @WhyNotHugo, loving this!

@WhyNotHugo
Copy link
Contributor Author

Thanks. I'm currently using trouble.nvim for two things:

  1. Diagnostics for current file.
  2. Find all references (via LSP as well).

I'm trying to figure out a way to use compact for the former, but not for the latter. Not sure how it can happen, I think I need to add a second param to :Trouble.

@folke
Copy link
Owner

folke commented Jul 27, 2021

Hi! Thank you for this PR. Sorry for the delay, but I'm traveling right now. I'll be back next week.

I have plans to allow custom grouping and also to disable grouping (which is basically what you added).

Would be great to change your pr to allow the following settings instead:

  • padding: defaults to 1. Can be set to 0 to disable as in your pr
  • group: defaults to true (will add other options like severity etc later.) Can be false to disable groups and simply list the results as in your pr.

Do you think this makes sense?

@folke
Copy link
Owner

folke commented Jul 27, 2021

@WhyNotHugo you can already pass options to Trouble. Either to the command with option=value, or by using the Lua api

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Jul 27, 2021

I think your approach makes a lot more sense. It's tidier, can do the same thing, and is a bit more flexible.

you can already pass options to Trouble. Either to the command with option=value, or by using the Lua api

Those aren't the options that get passed to renderer.render, are they?

If I wanted two mappings that open Trouble, one with padding=0 and another with padding=1, is this the right way to start? I'm not sure where to hold this value or pass it one.

Arguments passed to `:TroubleToggle` were being ignored. Process them
just like arguments passed to `:Trouble`.
@WhyNotHugo
Copy link
Contributor Author

Updated to implement group and padding.

I also noticed that :TroubleToggle wasn't passing opt to Trouble.toggle, but :Trouble does pass opts to Trouble.open. I've made this to now be consistent via 8a8cb37. Lemme know if that makes sense to you too.

@WhyNotHugo
Copy link
Contributor Author

Hint: git diff -w(hide whitespace changes) is easier to read in this case.

The "group" flags avoids grouping results, which is very useful when
only seeing results for a single file (where collapsing/expanding makes
less sense).

The "padding" flag controls whitespace around results, which also plays
well with "group=false" too.
@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Sep 2, 2021

Do you have any further feedback on this?

I've done some testing with TroubleOpen lsp_document_diagnostics group=false and TroubleOpen lsp_document_diagnostics, and both work as intended. The config value is used when nothing is explicitly passed.

@folke folke merged commit ff40475 into folke:main Oct 22, 2021
@folke
Copy link
Owner

folke commented Oct 22, 2021

Thank you!

@WhyNotHugo WhyNotHugo deleted the simpler-single-file branch November 12, 2021 15:27
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.

None yet

3 participants