Source location

[pranavkm]

Ties into aspnet/Mvc#2321

[pranavkm]

I'm not sure how attached we are to these operators - they are scarcely used in our code.

[pranavkm]

Also confirmed with Todd that these operators aren't used in tooling.

[pranavkm]

Will fix.

[dougbu]

was this fixed?

[NTaylorMullen]

• Add test for SourceLocationTracker verifying that the file paths flow with updated SourceLocations, just need to test the UpdateLocation method.
• Add test that validates codegen prioritizes SourceLocation.FilePath over the contexts source file.

[NTaylorMullen]

⌚

[dougbu]

what scenarios will continue to use this constructor? for example, could it be internal and used only in tests?

[pranavkm]

We use this one all over the place - I'm being pretty lazy in this PR and specifically using the other ctor when creating inherited Chunks in Mvc. The existing code continues to use this ctor.

[dougbu]

⌚

[pranavkm]

Updated

[dougbu]

• why doesn't the comparer handle null?
• adding the wrong value to the hash. either return Add(hashCode) or do what Add(object) does:

Add(hashCode);
return this;

[dougbu]

also, if the comparer can't handle null suggest making a similar change to Add(object)

[pranavkm]

Unfortunately the comparer performs a null check so this ends up throwing. I'll fix the hash code

[dougbu]

including the file in a relative location seems a bit nonsensical. one possibility would be for - to return a SourceLocation with FilePath==null (maintaining current comparison of input FilePath values) and + to throw if both SourceLocations have FilePath!=null, then return a new SourceLocation with the non-null FilePath (if any).

[pranavkm]

I'm neutral about this - subtraction isn't used anywhere in the codebase. Additions are used where the right operand is a relative location and it's used in calculating the offset from document start. So it sort of makes sense to preserve the document location in that case.

[NTaylorMullen]

⌚

[dougbu]

nit: no 's' in WriteLineNumberDirective_

[dougbu]

⌚

[NTaylorMullen]

Should investigate travis failures. It's pasing fine on dev branch.

[pranavkm]

Updated

[dougbu]

can't use [NotNull] here. breaks other constructor.

[dougbu]

⌚ mainly for (A - B) + C discussion

[pranavkm]

Updated.

[dougbu]

add , and neither is null

[dougbu]