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.

Sign up for GitHub

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

Jump to bottom

sourceRanges confusing in CallHierarchyIncomingCall and CallHierarchyOutgoingCall #81746

Closed
dbaeumer opened this issue Oct 1, 2019 · 3 comments
Closed

sourceRanges confusing in CallHierarchyIncomingCall and CallHierarchyOutgoingCall #81746

dbaeumer opened this issue Oct 1, 2019 · 3 comments
Assignees
@jrieken
Labels
api bug Issue identified by VS Code Team member as probable bug callhierarchy editor-symbols definitions, declarations, references polish Cleanup and polish issue verified Verification succeeded
Milestone
October 2019

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Oct 1, 2019

Testing #81655

The sourceRanges denote the range in the from item. However since the CallHierarchyIncomingCall has a source property this gets confusing. In addition source and target aren't easy to understand. For me a call goes from function A to function B.

I would opt to name them as follows:

	export class CallHierarchyOutgoingCall {
		fromRanges: Range[];
		to: CallHierarchyItem;
	}
	export class CallHierarchyIncomingCall {
		from: CallHierarchyItem;
		fromRanges: Range[];
	}
@jrieken jrieken modified the milestones: September 2019, October 2019 Oct 1, 2019
@jrieken jrieken added api editor-symbols definitions, declarations, references polish Cleanup and polish issue labels Oct 1, 2019
@dbaeumer
Copy link
Member Author

dbaeumer commented Oct 1, 2019

May be it would be helpful to explain that the graph is basically modeled as an adjacent matrix and that the API we provide gives compressed access (no empty elements) to the rows and columns of that matrix.

@dbaeumer
Copy link
Member Author

dbaeumer commented Oct 1, 2019

Basically it is a adjacent list (https://en.wikipedia.org/wiki/Adjacency_list)

@gregvanl
Copy link

gregvanl commented Oct 1, 2019

I found this confusing as well. I was going to propose CallHierarchyOutgoingCall parameters be target and targetRanges but I realized that I was interpreting it incorrectly.

@jrieken jrieken mentioned this issue Oct 3, 2019
Closed
@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Oct 7, 2019
jrieken added a commit that referenced this issue Oct 8, 2019
@jrieken
renames: source -> from, target -> to, #81746
46be30e
@jrieken jrieken closed this as completed Oct 8, 2019
@roblourens roblourens added the verified Verification succeeded label Oct 30, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jrieken jrieken

Labels
api bug Issue identified by VS Code Team member as probable bug callhierarchy editor-symbols definitions, declarations, references polish Cleanup and polish issue verified Verification succeeded
Projects
None yet
Milestone
October 2019
Development

No branches or pull requests
4 participants
@roblourens @jrieken @dbaeumer @gregvanl