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

#18696 Support line number fragments in vscode.open #19535

Closed

Conversation

Projects
None yet
6 participants
@charlespierce
Copy link
Contributor

commented Jan 27, 2017

Implements #18696

Added support for line / column number fragments to vscode.open command. The parsing logic was taken from the link opening service at vs/platform/opener/browser/openerService.ts.

It seems like this logic should be consolidated somewhere, but I'm still new to the project and I'm not sure where the best place to add this kind of shared logic would be, and how to access it from both places that need it.

@mention-bot

This comment has been minimized.

Copy link

commented Jan 27, 2017

@charlespierce, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bpasero and @egamma to be potential reviewers.

@bpasero

This comment has been minimized.

Copy link
Member

commented Jan 30, 2017

@jrieken would it not make sense to allow to pass in line/col information from the showTextDocument method? I think this vscode.open command is turning into a confusing parallel way for extensions to open documents, I think we should rather stick to one.

@jrieken

This comment has been minimized.

Copy link
Member

commented Jan 31, 2017

Yes, people should use showTextDocument and not the open command

@jrieken jrieken closed this Jan 31, 2017

@bpasero

This comment has been minimized.

Copy link
Member

commented Jan 31, 2017

@jrieken since there is no way of specifying the line/column information in the showTextDocument is the idea to just apply it from the callback in the editor?

@jrieken

This comment has been minimized.

Copy link
Member

commented Jan 31, 2017

that or a new PR that adds an options parameter with that information, like showTextDocument(doc: TextDocument, options: {revealPosition: Position})`. However, with the editor in hand the are much richer reveal options

@charlespierce charlespierce deleted the charlespierce:line_number_uri branch Jan 31, 2017

@joaomoreno

This comment has been minimized.

Copy link
Member

commented Feb 10, 2017

@bpasero @jrieken How about if you want to show a Diff editor?

@bpasero

This comment has been minimized.

Copy link
Member

commented Feb 10, 2017

@joaomoreno we need support to showTextDocument(diffResource), I never liked the command for opening. it was initially added to allow to open an editor in a new window I think and should not be used as general way of opening editors imho.

@jrieken

This comment has been minimized.

Copy link
Member

commented Feb 10, 2017

Not sure about diffResource but "yes" there are requests for exposing the diff editor in the API and some way to show a diff editor two resources is something we need

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.