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

Navigation from diagnostics panel to error location in view fails #24

Closed
deathaxe opened this issue Aug 19, 2017 · 6 comments
Closed

Navigation from diagnostics panel to error location in view fails #24

deathaxe opened this issue Aug 19, 2017 · 6 comments

Comments

@deathaxe
Copy link
Contributor

By double clicking into the diagnostics output panel, it is / seems intended to be able to navigate to the error location in the file. The view containing the file should be focused and scrolls to the error.

Didn't find the handler for this action right now. Maybe it is a default behavior of ST?

The issue with this functionality is - double clicking opens a new empty view instead, because the file path being used to find the view misses the drive letter.

screenshot

The reason is a wrong translation of the uri to a path relative to the project root. The following function assumes the common prefix of all folders in the sidebar to be this project root. This is wrong. You may add folders of any location to the sidebar and group them together as a single project. Therefore each view needs its own project root which is represented by exactly one element of window.folders().

def get_project_path(window: sublime.Window) -> 'Optional[str]':
    """
    Returns the common root of all open folders in the window
    """
    if len(window.folders()):
        folder_paths = window.folders()
        return os.path.commonprefix(folder_paths)
    else:
        debug("Couldn't determine project directory")
        return None

I temporarily ommited translation to relative path in update_diagnostics_panel(window), which fixes the navigation issue.

screenshot

There are 2 options:

  1. always show absolute paths in the output panels
  2. fix translation to relative paths (my favorite)
@tomv564
Copy link
Contributor

tomv564 commented Aug 20, 2017

Thanks for another well-documented report!
The go-to-result behaviour is default ST behaviour, and your fix to make the path absolute is a good workaround for now.

It sounds like you have folders from different drives in your project, in which case the common prefix should be broken. Is this correct?

@deathaxe
Copy link
Contributor Author

deathaxe commented Aug 20, 2017

It sounds like you have folders from different drives in your project, in which case the common prefix should be broken. Is this correct?

Yes I do. As it is possible to add any folder of any location, it should be supported by any package. Therefore I'd suggest to at most translate between absolute view.file_name() and a relative path as presented in the sidebar. I already wrote some functions to do that, but can't remember where at the moment.

@tomv564
Copy link
Contributor

tomv564 commented Aug 20, 2017

I agree we should support projects with multiple root folders in some form.

It means restructuring the plugin to change from:

  • 1 language server per window per language
    to:
  • 1 language server per window per root folder per language

Because we have to send a valid rootUri to every language server instance.

@deathaxe
Copy link
Contributor Author

A language server can handle only one project? Is this the reason for a language server to be started per window? I was wondering why there isn't just one global server instance, which provides information for all windows and files in it.

@deathaxe
Copy link
Contributor Author

The issue was caused by removing the "result_base_dir" setting from the output panel.

But nevertheless, this setting works globally for the output panel. If a panel displays results from multiple views with files from different folders, file navigation fails for all entries not being child of result_base_dir.

@tomv564
Copy link
Contributor

tomv564 commented Aug 20, 2017

I created a new issue exploring your usage scenario some more (#33)

As this cause of the problem was resolved, is it okay if we close this issue and continue in the new one?

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

No branches or pull requests

2 participants