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

online_editor: Code navigation between files #1709

Merged
merged 2 commits into from Oct 6, 2022
Merged

Conversation

hunger
Copy link
Member

@hunger hunger commented Oct 5, 2022

Enable code navigation between open files.

E.g. go to definition for "Homepage" in the printer demo's main page does work now. It used to open a new tab with the source code in it before.

Comment on lines +314 to +323
{ resource, options },
source: monaco.editor.ICodeEditor | null,
_sideBySide?: boolean,
): Promise<monaco.editor.ICodeEditor | null> => {
if (editor == null) {
return Promise.resolve(editor);
}

if (!this.set_model(resource.toString())) {
return Promise.resolve(null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think url would be a more readable name :). Maybe set_model could also be renamed to set_current_url since it does take a url as a parameter and only internally uses the models.

Suggested change
{ resource, options },
source: monaco.editor.ICodeEditor | null,
_sideBySide?: boolean,
): Promise<monaco.editor.ICodeEditor | null> => {
if (editor == null) {
return Promise.resolve(editor);
}
if (!this.set_model(resource.toString())) {
return Promise.resolve(null);
{ resource: url, options },
source: monaco.editor.ICodeEditor | null,
_sideBySide?: boolean,
): Promise<monaco.editor.ICodeEditor | null> => {
if (editor == null) {
return Promise.resolve(editor);
}
if (!this.set_model(url.toString())) {
return Promise.resolve(null);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that should change, but not as part of this patch: There are several methods that have Model in their name and they all should change.

I also want to use Url directly over converting that to and from string in several places.

@hunger hunger merged commit d8f6b78 into slint-ui:master Oct 6, 2022
@hunger hunger deleted the oe_nav branch October 6, 2022 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants