-
Notifications
You must be signed in to change notification settings - Fork 110
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
Use a dropdown setting to switch between the ways of visualizing inactive regions (background color vs. opacity) #643
Comments
I definitely agree a drop-down like the one you describe would be nicer! I just wasn't sure whether / how it can be done using VSCode's APIs / extension points. If this is possible to do, PRs to change it are very welcome, and I'm happy to review them.
A note on foreground color: one of the specific goals in #193 was to not set a foreground color on the entirety of an inactive region, so that the non-foreground style (background or opacity) that is set can be combined with foreground colors set by VSCode's client-side (TextMate) highlighting engine. This way, e.g. keywords in inactive regions still remain bold, strings are still colored differently than identifiers, etc. This makes the inactive code easier to read and edit than if it was all one color, while still making it clear that it's inactive. I suppose we could add an option to make it all one color if you think that would be useful, but we'd definitely want to keep an option to combine background or opacity styles with TextMate foreground colors.
I'm not following this one, could you elaborate please? |
I understand why keeping the foreground colors was desired, and I guess most people would want that. But just for the weirdos like me, a one color foreground would be nice. The styles (bold, italics etc.) would likely leak in (like in QTC screenshot), but it's not big of a deal, in my opinion. Of course, other options, like opacity and background, should still be available to others that prefer them.
I'd happily look into that, but I'm no node.js and/or vs-code extension expert or even a beginner. I tried creating an extension some time ago, not related to this issue, and I just dropped it because of difficulties of learning a new language and pipeline. Not to mention expanding on an already existing big extension someone had already made. That seems dreadful for me.
VSC has a built-in C/C++ extension, which has inactive region tweaking options: They seem to do the same thing clangd does with its opacity and background options for inactive code. But it only works if IntelliSense is enabled, which is not, if we're talking clangd. What I'm suggesting is to add an option which, instead of using clangd's own settings for background and opacity, would rely on already existing settings from this built-in extension. No additional settings needed, just use these values. I'm not sure if extensions can access other extensions' settings, but if they can, this seems pretty easy to implement. |
Understood. I think a possible conclusion here is "a dropdown where different options have different associated data types (e.g. one option requires selecting a color, another option requires setting an opacity value)" is not a type of setting that the VSCode extension API supports.
Thanks, I understand now. While I haven't been involved in vscode-clangd from the beginning, my understanding is that the project has made a deliberate choice not to try to use the same settings as vscode-cpptools. For example, we do not pay any attention to |
Ah, I see. Then, this option should not exist. But still, these settings might be a good example of how to organize inactive region highlighting. These four settings are enough to achieve everything I have proposed, without the use of the drop-down menu. Btw, why was this choice made? I don't see any particular reason not to base a C/C++ extension off of a built-in C/C++ extension. Or am I reading to much into this and this only affects settings?
I don't understand, how are different types a part of this? |
Just to clarify, vscode-cpptools may be the extension for C/C++ code that VSCode recommends by default (presumably because both VSCode itself and vscode-cpptools are authored by Microsoft), but it's not "built-in" in the sense that it's bundled with a base VSCode installation, or that its presence can be assumed by other extensions. vscode-cpptools and clangd are basically alternatives to each other. Both provide a language server and associated editing features. vscode-cpptools is proprietary (at least the language server is), and clangd provides an open-source alternative. As such, I don't think there's any meaningful way in which clangd could be "based on" vscode-cpptools. At most, it could try to respect vscode-cpptools' settings. The decision not to do so pre-dates my involvement in the project, but I can speculate as to the reasons:
Maybe I misunderstood the suggestion; I thought you meant having a dropdown where e.g. some (but not all) options include a color picker contained within them.
Well, those look fairly similar to ours, down to the notes like "This setting only applies when inactive region dimming is enabled." The main difference is that they also have an option for a foreground color, so it seems like that's the main ask here. |
I understand now. I must've confused the ms-vscode.cpptools for the vscode.cpp, which is actually built-in and called C/C++ Language Basics. I somehow assumed that the settings I was talking about are from the Basics one, while they are actually not, the Basics one has no settings whatsoever. Sorry for that. Now it actually makes perfect sense why the decision in question was made, and I actually respect it.
Not really relevant to the subject of the issue (as pretty much this whole tangent regarding the cpp extension), but I thought ms-vscode.cpptools was pretty much mandatory for any C/C++ development on VSC. And now I actually see that clangd replaces it, the only thing clangd doesn't seem to support is debugging. The more you know, I guess.
What I meant is a menu like this one from Error Lens which let's user pick what to display in editor gutter, and additional options to fine tune how to display it: To my mind, something similar to this would be more user-friendly. Instead of deducing what different options do, it's easier to pick an 'operation mode' of a certain feature, a behaviour, if that makes sense. And then user can fine tune it with some settings, like color, opacity etc. This does not involve any trickery with different types and seems fairly simple to implement. This was my main point in the original comment (and of course foreground option).
True. I regarded this style as a 'back-up plan', if the drop-down menu idea was rejected. One problem, however, is that clangd's |
Ok, thanks for clarifying! If we're just talking about replacing the checkbox ("use background highlight rather than opacity") with a dropdown with two options ("background highlight" and "opacity"), that should be straightforward to change.
I think the idea behind making it a workbench color customization is that themes can specify a value for it, so that when you change themes, the background color can potentially also change in a way that's harmonious with the other colors in the theme. Do you know how Microsoft's extension deals with this? |
Adding a foreground highlight should also be easy, although I'm not sure if it's anything similar to adding a background highlight.
I was thinking about leaving it a color customization but also adding a separate setting, which would override the color customization. For example, when no value is specified, use theme color, otherwise use the value specified. Maybe leave just the color customization without a separate setting, but specify in the hint to the drop-down menu option, like
Doesn't seem that Microsoft's extension creates a workbench color customization for this. I can't really think of a reason why. |
Thanks. To summarize, the proposal is for two changes:
Both of these sound reasonable to me. I'll update the title of this issue to track the first one, and file a follow-up issue to track the second one. |
#654 is the second issue |
Initially started as this question of mine, but I think it belongs as a feature request. Similar thing has been already implemented, but it seems a bit crude in my opinion. I think it should be handled like a drop-down menu with options rather than multiple different settings pointing to each other like
used only if xxx
. Options may include:disable
- no change at all (like currentopacity=1
,useBackground=false
);opacity
- just opacity (opacity=0.55, useBackground=false
);workbenchColors
- uses currently presentclangd.inactiveRegions.background
and maybe create another one for foreground? An example would be this picture from the question about how Qt Creator does that:workbenchColorsOpacity
- likeworkbenchColors
but also utilizes opacity setting;cppBuiltInColors
- use already existing options from built-in cpp extension (like this answer shows, but sadly doesn't work for clangd).Not saying all of these should exists, but I would be glad to see some setting to set the overall foreground of inactive region, either in workbench color customizations or as a separate setting. I really like how it looked in QTC. Shouldn't be too hard to implement, right? We already have working drop-down menus and almost all options already exist.
The text was updated successfully, but these errors were encountered: