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

don't strip comments #24

Closed
agnewp opened this issue Jul 15, 2019 · 5 comments
Closed

don't strip comments #24

agnewp opened this issue Jul 15, 2019 · 5 comments
Labels
new feature New feautre request

Comments

@agnewp
Copy link

agnewp commented Jul 15, 2019

comments are used as descriptions in the older syntax and you are effectively removing all self documentation capability from GQL. I find myself wanting to use this but not being able to.

@agnewp agnewp changed the title don't strip comments, that is the don't strip comments Jul 15, 2019
@mununki
Copy link
Owner

mununki commented Jul 16, 2019

@agnewp Just to know clear, Is it the same issue to #23? If not, can you give me a sample code to reproduce it?

@agnewp
Copy link
Author

agnewp commented Jul 16, 2019

this is only slighly related to issue 23, but NOT the same

to reproduce, add a comment line to any file and the post-merge result does NOT include that comment line
in a slightly older version of the GQL spec, comment lines next to the fields are used as description meta data. the spec parser i am using supports both this comment line syntax as well as what is mentioned in #23.

so to summarize, if you simply strip out comments when merging GQL files, you might actually be stripping out things that are meant to be self documentation.

does that make sense?

@mununki
Copy link
Owner

mununki commented Mar 31, 2022

Sorry for the huge delay. It totally makes sense to me.

Actually, gqlmerge is using a go scanner for lexing and parsing the GraphQL SDL. So the description is tokenized as a string first. In order to resolve this, the best way is to make a custom scanner to tokenize. It will take some time to fix it. #31

@mununki mununki added the new feature New feautre request label Aug 1, 2023
@mununki mununki mentioned this issue Aug 6, 2023
5 tasks
@mununki
Copy link
Owner

mununki commented Aug 9, 2023

Sorry for the huge delay, I've released a fixed version that doesn't strip descriptions and comments. Can you test with https://github.com/mununki/gqlmerge/releases/tag/v0.2.9?

@mununki
Copy link
Owner

mununki commented Aug 10, 2023

As of v0.2.9, it's been fixed to not strip description and comment, so closing the issue. If you have any issues, please reopen it.

@mununki mununki closed this as completed Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feautre request
Projects
None yet
Development

No branches or pull requests

2 participants