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

Goto definition on aliases #1462

Open
thomasaarholt opened this issue Feb 13, 2024 · 6 comments
Open

Goto definition on aliases #1462

thomasaarholt opened this issue Feb 13, 2024 · 6 comments

Comments

@thomasaarholt
Copy link

Hello again! I'm still loving using this as an LSP with helix!

Would it be possible to add support for goto definition on aliases? We typically include upstream models at the top of our models inside CTEs, and then join them further down. In the example below, it would be really nice to be able to position the cursor on the alias foo, call goto definition and be taken to foo as, or position on f. and be taken to as f.

with foo as (
    select * from {{ ref('foo') }}
),

bar as (
    select * from {{ ref('bar') }}
)

select
    f.id,
    f.name,
    b.address
from foo
left join bar on f.id = b.id
@pgrivachev
Copy link
Collaborator

Hey @thomasaarholt!

From what I gather, you're looking to enable a "go to definition" command that, when executed with the cursor on foo, moves the cursor to the start of the foo CTE. This seems feasible, and I'll look into it.

We currently support navigation to CTEs, but this is limited to columns. Could you verify whether the navigation demonstrated in the video (going to the definition for f.id, f.name, and b.address) functions within Helix as well?

I used the following SQL:

WITH foo AS (
  SELECT 1 AS id, 'Name1' AS name UNION ALL
  SELECT 2, 'Name2'
),

bar AS (
  SELECT 1 AS id, 'Address1' AS address UNION ALL
  SELECT 2, 'Address2'
)

SELECT
  f.id,
  f.name,
  b.address
FROM foo f
LEFT JOIN bar b ON f.id = b.id;
reproduce.mp4

@thomasaarholt
Copy link
Author

thomasaarholt commented Feb 19, 2024

Hi @pgrivachev!
That's an accurate description of what I want :)

Here's a repro of your excellent video, but in helix. I first try your suggested file, pressing gd (goto definition) on the various fields. This does not work. Then I open one of my dbt models, and show that gd works within a dbt/jinja ref function (proving that the lsp is working).

Screen.Recording.2024-02-19.at.09.32.19.mov

I notice in :log-open that the following line is produced every time gd is performed on the selected fields in your sql example:

2024-02-19T09:32:40.127 helix_term::application [WARN] Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server

@thomasaarholt
Copy link
Author

thomasaarholt commented Feb 19, 2024

Actually, that line in the logs might be a red herring, and does not appear with every gd call after all. Instead, immediately when I open any sql file, four identical lines are printed:

2024-02-19T09:58:30.650 helix_term::application [WARN] Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server
2024-02-19T09:58:30.650 helix_term::application [WARN] Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server
2024-02-19T09:58:30.650 helix_term::application [WARN] Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server
2024-02-19T09:58:30.650 helix_term::application [WARN] Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server

I'm opening a new issue since I'm now getting a node error after experimenting a bit with goto definition.

edit: I think what happened with the node error is that I got blocked out of duo mobile for 15 min, since I didn't notice that my phone kept getting MFA notifications. Everything works again now.

@pgrivachev
Copy link
Collaborator

Hi @thomasaarholt!

I also tried to reproduce the issue in helix and everything is working for columns in my case (attached the video). It is strange that go to definition doesn't work in your case. Could you please clear LSP logs then reproduce the issue and send me resulting logs?

reproduce.mp4

@thomasaarholt
Copy link
Author

Interesting!

Here are the logs using helix with four different logging verbosity (from hx test.sql to hx -vvv test.sql).

hx dbt-models/run.sql
hx -v dbt-models/run.sql
hx -vv dbt-models/run.sql
hx -vvv dbt-models/run.sql

Including a few bits of info in case:
image

@pgrivachev
Copy link
Collaborator

@thomasaarholt thank you for detailed logs!

According to current design this navigation should start working only after whole project analysis, that could take some time. So my assumption that if you wait some time (depending on your project size) the go to definition feature should start working. When it is don you should see the following LSP log:

2024-02-24T22:59:58.375 helix_term::application [INFO] window/logMessage: LogMessageParams { typ: Log, message: "snowflake 2024-02-24T21:59:58.374Z:  Project analysis completed" }

Please let me know if this helps.

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