justcla / HotCommands

A collection of commands, analyzers, and refactorings for enhanced productivity in Visual Studio IDE
https://marketplace.visualstudio.com/items?itemName=JustinClareburtMSFT.HotCommandsforVisualStudio
51 stars 17 forks source link

Support for C/C++ #53

Open midurad opened 7 years ago

midurad commented 7 years ago

This is not really an issue, just a question. The extension is great by the way and it's an awesome addition to C# editing experience but I was wondering how difficult would it be to make it compatible with C/C++ or what the challenges are to make that happen.

Thanks!

justcla commented 7 years ago

Hmmm.... Good question. I wonder that, myself. Augustin is on the project and is the Microsoft C++ guy. I'll see if he knows.

midurad commented 7 years ago

Thanks a lot!

AugP commented 7 years ago

Hi midurad, I would love to see these features implemented for C++. Unfortunately I don't have a lot of free time to contribute presently, but as the extension is open source I hope some people take up the task. It's worth noting that some features already work across all languages though, like Duplicate Code.

tamlin-mike commented 7 years ago

I have started working on a context-free (that does not depend on the Analyzer packages) ExpandSelection, so it even works without a Workspace. It's in the branch tamlin-dev in my fork. It's currently a little rough around the edges, but at least the get the most important (IMHO) job done.

@AugP, To get feature parity with C# it'll have to be able to piggyback on the C/C++ "fast" parser for proper syntax context. Suggestions?

justcla commented 7 years ago

Thanks @tamlin-mike. I'll go have a look. Much appreciated.

justcla commented 7 years ago

I pulled your branch and ran the new implementation. I found it still "skipped" the "next" selection too much. Also, I could not get the Decrease Selection to work. Did you disable that?

tamlin-mike commented 7 years ago

The branch re-enabled the CTRL+W override (file-local macro-controlled, see top of ExpandSelection.cs) since I found that easier while running in the experimental instance of VS. Perhaps that's interfering? I didn't touch the Shrink code (or paths).

Yeah, it's definitely not perfect - and can never be, since it's completely context-free. But it can at least expand to select whole ""-enclosed strings, or the other delimiters I felt were the most important. I explicitly opted out of lt/gt (for template/generics args), since that would interfere way too much with comparison operators.

Additionally, there's a reasonably complete description of the intended behavior in the comment block of the handler.

justcla commented 7 years ago

Handling whitespace and braces is problematic. It would be good to get that part right.

tamlin-mike commented 7 years ago

From my testing it seems to be working as intended. For me personally, the most important thing is to be able to get strings and ()-delimited stuff properly selected, to simplify refactoring (since the VC++ part of VS hasn't (yet) got anything in that area). That is; I wrote the code to scratch a C++ editor itch, and in the process it got extended to handle more common delimiters too. That I found HotCommands dereferencing null was really just a side-effect of it. :-)

As such, I don't see a problem with WS handling. For more complex stuff, like multi-line ""-strings auto-concatenated-by-compiler it would obviously fail miserably without language syntax context, or at least some specific handling for a few different languages. But that's also way out of scope.

I was initially thinking of doing it regexp-style, which could maximize flexibility, but at the same time I wanted minimum resource usage (at this point), and I don't even know if C# regexp handler has a non-greedy qualifier (think PCRE).

For a completely context-free (i.e. knowing nothing about the language it's parsing) Expander, for programming languages, it's at least a start. Ultimately I think this could be a template for a worst-case fallback for expansion, where no syntax providers are available, but it's definitely not something to depend on if a syntax provider is available.

That said, please elaborate on the WS part. You think some surrounding WS should be included in some instance I hadn't considered?

For the actual C++ stuff, I suspect the best course of action is to await (C# :-) ) some words from Augustin, and take it from there. Who knows, maybe we're looking at a C# adapter to the C/C++ parser interface (to match the C# analyzer well enough to have a single codebase).

justcla commented 7 years ago

In order for this to work nicely, we really need it to be language-aware, which is why we implemented the Roslyn Syntax stuff. Being a basic context-free text expander was a non-goal. So, I think we should focus on the C++ and C# implementations.

When expanding beyond a single line, I feel it should select all lines in the scope (as defined by braces, typically) - but should not select the outer-most whitespace. The next expansion should select the white space and the braces that contain it. At the moment, it immediately expands to get all the whitespace within the scope, and the next invocation expands well outside the scope (often two-scopes out).

tamlin-mike commented 7 years ago

I agree 100% it should be language aware, but when that is made impossible by f.ex. design of the analyzers interface (which seems to require a Workspace), I felt this was better than nothing. It has already made my life easier in the C++ source-code editing area.

An earlier commit only handled single-line. Perhaps it's better to limit this "last-resort" to single-line (I'm definitely not opposing it - it really became multi-line by accident!).

I just wanted something instead of nothing for editing C++ source code.

justcla commented 7 years ago

I understand.

AugP commented 7 years ago

I had a chat with some people on the C++ team. Unfortunately it doesn't look like our parsers are extensible beyond top level information (definitions/declarations), so extension authors would have to essentially build their own (the way Visual Assist and ReSharper have done for example). We don't expose anything happening in function bodies to extensions, and enabling this would be complicated.

In general our extensibility points aren't as sophisticated as Roslyn, but that's because .NET basically had a chance to "re-invent the world" when they created Roslyn in the first place and they took advantage of these opportunities. It would be a multi-year investment from our engineering team to sit down and re-design everything so we would have a similar experience, and if we did so we would have to drop feature work that we would otherwise be doing. As a result it's not likely for this to happen unfortunately. We would love to do it and we've talked about it multiple times in the past, but it's just too costly of a project to consider at this time.

Sorry for not having better news :(.

On the plus side though, we can enable these features from our end. I am a huge fan of expand/contract selection (by the way good work on this @justcla!), and since I work in the C++ productivity space, I can certainly "influence" things a bit so it is higher on our backlog. But I don't know how soon we'd be able to release something like this since we work on so many different projects that it's hard to fit in new stuff in a timely manner.

tamlin-mike commented 7 years ago

I really can't wrap my head around the differences here:

1.

We don't expose anything happening in function bodies to extensions, and enabling this would be complicated.

2.

It would be a multi-year investment from our engineering team to sit down and re-design everything so we would have a similar experience

3.

On the plus side though, we can enable these features from our end.

They don't add up.

What I requested was access to the AST, with an optional hint towards "something more", but the FE AST is really the only thing actually needed. Everything else can be done from that. From simple selection modifications to complex transformations.

The C/C++ editor already has this info (and more), else stuff like code folding, keyword highlighting, squiggles (both syntax and semantics checking) etc would never work. So that the information is available in (generally) near-realtime is clear.

It seem idiotic if one would have to have to (re-)implement a whole C and C++ parser (again) just to get the info already produced by the tooling chain, especially at this time - Roslyn is almost completely open, but C++ is the complete opposite.

Is the problem of providing the (EDG-generated) AST from cpfe of technical or political nature?

But I don't know how soon we'd be able to release something like this since we work on so many different projects

Looking at the progress in the VS "C++ productivity space" from 1997 to 2017, I believe it could be prudent to recommend not holding breath. :-)