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

Add fragment support #44

Closed
wants to merge 2 commits into from

Conversation

bglusman
Copy link
Contributor

@bglusman bglusman commented Nov 7, 2024

I'm a little conflicted about how clean a change this is, but, it does what I wanted and maybe it's a useful part of deciding if the feature is useful to the library in general (or if there's some more general metadata or other changes to bias towards instead if other usecases for function metadata make sense, since this feels a little awkward and tightly coupled to pass just one bit of information through for each function)

I thought I had a (maybe marginally cleaner?) different way to define the final fragment_functions callback using a module attribute with accumulate: true but for some reason that wasn't working, so landed on this direct definition.

At the moment it incorporates the commit from my other PR #40 since they both touch the lexer, that's obviously a smaller change though and not tightly coupled.

In any case, attempts to address #41

Comment on lines +152 to +159
fragment_names = for {name, _docs, _fragments, true} <- metadata, do: name

fragment_func =
quote do
def fragment_functions(), do: unquote(fragment_names)
end

[fragment_func | Enum.reverse(acc)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we might also want to add

Suggested change
fragment_names = for {name, _docs, _fragments, true} <- metadata, do: name
fragment_func =
quote do
def fragment_functions(), do: unquote(fragment_names)
end
[fragment_func | Enum.reverse(acc)]
fragment_names = for {name, _docs, _fragments, true} <- metadata, do: name
fragment_func =
quote do
def fragment_functions(), do: unquote(fragment_names)
end
query_names = for {name, _docs, _fragments, false} <- metadata, do: name
query_func =
quote do
def query_functions(), do: unquote(query_names)
end
[query_func, fragment_func | Enum.reverse(acc)]

so that we have a direct source of all the non-fragment functions also, that might actually be cleaner for my use case because I also have to filter out __db_runner__, __db_options__ and run to skip explaining those, and with this I could JUST call explain on each function returned by query_functions 🤷‍♂️ just a passing thought, dunno, I don't really need both but might be worth having both.

@bglusman
Copy link
Contributor Author

bglusman commented Nov 8, 2024

Thinking about it a little more, another idea in this direction could be to call the new comment -- meta: or something and take a list of "tags", just take the string and split on spaces,and build a list of all functions with that tag, under that function name, or all returned in a map as a list under that key perhaps... just throwing it out here to see if that's a more appealing direction for this general idea? 🤷

@bglusman
Copy link
Contributor Author

given your comments in #41 I'll close this, the docs trick is decent, a bit less explicit, but nice to have that increase simplicity I guess.

@bglusman bglusman closed this Nov 13, 2024
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.

1 participant