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

feat: Implement the go line compiler directive #476

Conversation

af-md
Copy link
Contributor

@af-md af-md commented Jan 29, 2024

No description provided.

@af-md af-md force-pushed the implement_the_go_line_compiler_directive_in_generated_files branch from 0442f44 to ec733e3 Compare January 31, 2024 10:39
@af-md af-md force-pushed the implement_the_go_line_compiler_directive_in_generated_files branch from ec733e3 to 55837d9 Compare February 1, 2024 11:14
@af-md af-md force-pushed the implement_the_go_line_compiler_directive_in_generated_files branch from 55837d9 to 290c01e Compare February 2, 2024 11:26
@joerdav
Copy link
Collaborator

joerdav commented Jul 4, 2024

I think we didn't understand the other impacts that this change would have fully, I think we should close this for now and maybe revisit some other options.

@joerdav joerdav closed this Jul 4, 2024
@embiem
Copy link

embiem commented Aug 16, 2024

@joerdav could you please share what these impacts are? I'm looking into the lsp goToDefinition issue again and currently it seems like this is unresolved and not actively worked on. I'm willing to dig into this.

The discussion in the related go issue is also open with last status being that it's waiting on this line directive addition, which is now closed.

@joerdav
Copy link
Collaborator

joerdav commented Aug 19, 2024

Absolutely. The main issue is that the go line directive affects the debug information at runtime for code, my concern was that I actually don't know what else this would affect.

I know that for any panics originating from a templ components the stack traces would have line numbers that are pointing to completely the wrong place, and be missleading.

If we were happy with this and knew what other effects there would be, then I think I'd be comfortable merging, because this issue though small has quite a high demand.

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.

3 participants