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: parse HTML element and attribute positions #548

Merged
merged 12 commits into from
Mar 11, 2024

Conversation

alehechka
Copy link
Contributor

@alehechka alehechka commented Feb 23, 2024

Overview

This PR adds positions to the templ parser results for HTML elements and attributes relating to conversation in #498, specifically the recommendation made in #498 (comment).

This handles only calculating expression positions. I plan to open another PR when ready to use the calculated positions to enable LSP intellisense with a few hard-coded results. After that, I can work towards the solution I recommended in #498 (comment), to import an HTML LSP's data into templ's LSP.

@alehechka alehechka changed the title Feature/html element positions Parse HTML element and attribute positions Feb 23, 2024
parser/v2/types.go Outdated Show resolved Hide resolved
@alehechka alehechka marked this pull request as draft February 23, 2024 03:56
@alehechka alehechka marked this pull request as ready for review February 24, 2024 02:30
@alehechka
Copy link
Contributor Author

@a-h, I finished getting all the unit tests updated to reflect the position calculations so this should be ready for code review now.

@joerdav
Copy link
Collaborator

joerdav commented Feb 28, 2024

My immediate thoughts here are that maybe Expression isn't the right type, so far I think we only store Go in here.

I could be being nit-picky but I think it makes more sense for there to be an HTML specific expression type.

@alehechka
Copy link
Contributor Author

@joerdav, that sounds valid. I mainly reused it since it was already a pattern for storing positions. Should be simple enough to do a find and replace with a new name. Do you have anything in mind as an alternative to Expression?

@alehechka
Copy link
Contributor Author

Considering that Expression.Value ends up with the same value as Name from the parent struct in all of these cases, we could probably just use Range directly to store the positions of elements/attributes.

@alehechka
Copy link
Contributor Author

@joerdav, I went ahead and replaced Expression with Range in all cases. Let me know if this approach works better with you.

@joerdav
Copy link
Collaborator

joerdav commented Mar 5, 2024

@alehechka the type range makes sense, happy with that. I wonder if a better name for the property could be in order? The name Range Range to me suggests it's the range of the whole open tag, maybe NameRange Range could be more specific? After that I'm happy with this PR if @a-h is.

@alehechka
Copy link
Contributor Author

@joerdav @a-h
I updated those fields to NameRange as recommended.

@alehechka alehechka changed the title Parse HTML element and attribute positions feat: parse HTML element and attribute positions Mar 6, 2024
@a-h a-h merged commit 22f761a into a-h:main Mar 11, 2024
4 checks passed
@alehechka alehechka deleted the feature/html-element-positions branch March 11, 2024 12:54
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