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

Support for C/C++ #53

Open
midurad opened this issue Dec 7, 2016 · 14 comments
Open

Support for C/C++ #53

midurad opened this issue Dec 7, 2016 · 14 comments

Comments

@midurad
Copy link

midurad commented Dec 7, 2016

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
Copy link
Owner

justcla commented Dec 7, 2016

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
Copy link
Author

midurad commented Dec 8, 2016

Thanks a lot!

@AugP
Copy link

AugP commented Dec 13, 2016

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
Copy link

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
Copy link
Owner

justcla commented Apr 5, 2017

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

@justcla
Copy link
Owner

justcla commented Apr 5, 2017

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
Copy link

tamlin-mike commented Apr 5, 2017

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
Copy link
Owner

justcla commented Apr 5, 2017

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

@tamlin-mike
Copy link

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
Copy link
Owner

justcla commented Apr 5, 2017

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
Copy link

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
Copy link
Owner

justcla commented Apr 5, 2017

I understand.

@AugP
Copy link

AugP commented Apr 6, 2017

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
Copy link

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

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

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

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. :-)

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

No branches or pull requests

4 participants