Refactor how we're doing semantic highlighting and highlight left side of editor #47
Conversation
There was a problem hiding this comment.
Hey @JacobBovee , thanks for the PR. Can we have a discussion to help me understand why those newly added values? By reading them, I am concerned that the data structure might not be the best for consistency but I want to hear your intentions and reasons for making those changes.
| /// Generic types. | ||
| /// </summary> | ||
| /// <typeparam name="string"></typeparam> | ||
| public IEnumerable<string> RawGenericParameterTypes{get; init;} = Enumerable.Empty<string>(); |
There was a problem hiding this comment.
What is the difference between RawGenericParameterTypes compare to GenericParameterTypes?
There was a problem hiding this comment.
As I mentioned below this simple includes the unprocessed GenericParameterTypes so that the pre-beautified stack can be highlighted correctly. The idea is to piggyback off the parsing we're doing for the beautification, otherwise we'd be essentially re-parsing the callstack from our extension to determine the semantic highlighting rules.
It could certainly be more intuitive though, maybe all of the parsed entities should simple return a Beautified{Entity} and a Full{Entity} where Entity is a generic parameter type, method param, etc.
| @@ -3,5 +3,5 @@ namespace NetStackBeautifier.Core | |||
| /// <summary> | |||
| /// A parameter. | |||
| /// </summary> | |||
| public record FrameParameter(string ParameterType, string ParameterName); | |||
| public record FrameParameter(string ParameterType, string ParameterName, string RawParameterType, string FullParameterName); | |||
There was a problem hiding this comment.
What is FullParameterName? How is it different than ParameterName? Why do we need them?
There was a problem hiding this comment.
We do some transformations on the parameter names depending on the types. For example int32 becomes simply int. This ensures that the unprocessed value is returned so that the starting call stack can be highlighted correctly.
|
Just bookkeeping, @JacobBovee & I had a offline discussion. We agreed on a better way to preserve the original values for IFrameLine objects, and there will be an iteration on the implementations. |
Much cleaner method of running filters for determining semantic highlighting. Also introduces support for "full stack" highlighting which allows us to highlight the initial text editor we've come into contact with. Part of this includes the backend service sending over the parsed tree of raw properties. Gonna look at a nicer way of doing this.