Skip to content

Conversation

@mstephen19
Copy link
Contributor

No description provided.

@mstephen19 mstephen19 changed the title Add GraphQL language to Markdown feat(markdown): add GraphQL Jul 18, 2022
@mstephen19 mstephen19 requested a review from B4nan July 18, 2022 10:32
@mstephen19
Copy link
Contributor Author

Also added quote_type to .editorconfig

Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

Would be nice to separate code style changes into separate PR, as its hard to see what you actually changed and what is just code formatting. Also there are some trailing commas missing and lint step fails.

TBH those line breaks look like from prettier. And prettier does quite a poor job in adding line breaks if you ask me :]

if (token.type === 'code' && token.lang) {
if (token.lang === 'marked-tabs') {
codeTabsObjectPerIndex[markedTabTokenIndex] = codeTabObjectFromCodeTabMarkdown(token.text);
codeTabsObjectPerIndex[markedTabTokenIndex] =
Copy link
Member

Choose a reason for hiding this comment

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

like this line, why are you breaking it? my guess is prettier and their dumb 80 chars max line length default? :]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@B4nan yeah sorry that's prettier. I have auto-save when a window is unfocused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I have the line length limit to 140

Copy link
Member

Choose a reason for hiding this comment

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

Using any auto-formatting on projects that does not provide configuration for the tool is... Let's say not very polite :] Even if we want to change code style, it can't be part of such a PR (that should have pretty much one line diff), it needs to be a separate one.

But I have the line length limit to 140

That line had 107 characters 🤷

@fnesveda
Copy link
Member

I was going through old, forgotten PRs to clean them up, and I decided to finish this one, since it's tiny but useful.

@fnesveda fnesveda merged commit a7405fe into master May 29, 2023
@fnesveda fnesveda deleted the add-gql branch May 29, 2023 08:58
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