Symbol uses

[rojepp]

Implements #31.

I botched the previous PR by rebasing, so git wouldn't let me push (when will I ever learn).
I added a few fields from SymbolUse, including IsFromDefinition which should address @7sharp9's concern. Anything else that should be included?

[7sharp9]

I want to click the Merge Pull Request but Ill leave that to @rneatherway :-)

[rneatherway]

Adding 1 here doesn't seem right, they should already be 1-based from the Alternate syntax. Looking at the results I also think they should end one earlier e.g. with:

let testval = FileTwo.NewObjectType()

The results have a range with columns start=5 and end=12. t is definitely at 5, and l is 6 further on, so 11. This is an inclusive range and I think we should leave it alone, documenting it as such (if we ever get around to adding documentation for this project!). If editors prefer to work with an exclusive range they can make the adjustement locally.

[7sharp9]

XS columns are 1 based we do something like:

        let start, finish = Symbols.trimSymbolRegion symbolUse lastIdentAtLoc
        let filename = doc.FileName.ToString()  
        let offset = doc.LocationToOffset(start.Line, start.Column+1)
        let domRegion = DomRegion(filename, start.Line, start.Column+1, finish.Line, finish.Column+1)

        let symbol = createSymbol(doc, context, symbolUse.Symbol, lastIdentAtLoc, domRegion)
        let memberRef = MemberReference(symbol, filename, offset, lastIdentAtLoc.Length)

        //if the current range is a symbol range and the fileNameOfRefs match change the ReferenceUsageType
        if symbolUse.IsFromDefinition then
            memberRef.ReferenceUsageType <- ReferenceUsageType.Write

        memberRef

[rojepp]

I'm not sure how to fix this. FCS seems to report 0-based columns (lines are 1-based). Do you want me to only add 1 to StartColumn? That doesn't seem right to me. That would leave testva highlighted.

[rneatherway]

Sorry, I misread. It seems that FCS accepts 1-based column numbers but returns 0-based. Why would this possibly be a good idea?

I've looked at the output we give and I think only declarations, finddecl and this new command return positions. I think they should all return 0-based or all return 1-based columns, for consistency. However, I think that if FCS is outputting 0-based columns it should probably be accepting 0-based as well.

[rojepp]

It is all very confusing. Adding 1 to StartColumn and EndColumn seems to be the right thing to do, so we normalize to consistently being 1-based . Do the other commands return 0-based?

[rojepp]

Find Declaration seems to return 0-based.

[rneatherway]

Yes, I guess you posted at the same time as me ;-)

I would be happy if everything returned 1-based so at least FSAC is consistent even if FCS isn't.

@7sharp9 do you adjust the results you get from GetNavigationItems for the pathed document support as well to be 1-based?

[rojepp]

Let's do it. :)

Separate PR or this one?

[rneatherway]

Here is fine by me.

[rojepp]

Done.

[rneatherway]

Otherwise looks great. Thanks so much for the contribution, things are really coming on!

[rneatherway]

The JSON printing just below Data =errs is also affected

[rojepp]

Good catch, I'll fix it shortly.

[rneatherway]

Having to also correct the errors makes it less enjoyable :-(

[rojepp]

Argh, FSharpErrorInfo has no public constructor. I guess the only option is to make a record type with the same fields.

[rojepp]

In doing a record type, should I keep StartLineAlternate/EndLineAlternate? Maybe a good time to expose fewer fields?

[rojepp]

With that, I think I'm done. :)