jparise / vim-graphql

A Vim plugin that provides GraphQL file detection, syntax highlighting, and indentation.
MIT License
490 stars 25 forks source link

use a default shiftwidth of 2 #17

Closed timfeirg closed 6 years ago

timfeirg commented 6 years ago

I can't seem to find the coding style guide for graphql, perhaps they just don't have one.

But they are using a 2 space indentation through out their websites, like http://graphql.org/learn/queries/.

So I figured it'd be nice of vim-graphql to apply the official style as the default.

jparise commented 6 years ago

I think this makes sense. It's also the indentation style used by GraphiQL.

Why add these in after/index/graphql.vim and not in ftplugin/graphql.vim though? after/ makes sense if you're adding this locally (after the graphql plugin has loaded), but because this addition would be to the plugin itself, I think ftplugin/graphql.vim is clearer. Am I missing a use case?

timfeirg commented 6 years ago

I've put those in ftplugin/graphql.vim.

I know nothing about vim plugin development, I just saw python-mode do this and assumed after directory is the correct place to do this.

Thanks for this knowledge and your quick response.

jparise commented 6 years ago

Looks great, and I appreciate your contribution. Thanks!

jparise commented 6 years ago

@timfeirg, now that I think about it, I don't think there's any reason to set tabstop=2. shiftwidth and softtabstop should cover all of the "normal" cases, and I don't see any harm in letting "tabs be tabs". What do you think about not setting tabstop?

timfeirg commented 6 years ago

Actually I didn't give this too much thought and just copied this config from https://github.com/python-mode/python-mode/blob/develop/after/indent/python.vim#L6.

But I think it's because we're already expanding tabs here, so it's already "tabs cannot be tabs", might as well set tabstop to be equal to shiftwidth?

jparise commented 6 years ago

The case I'm thinking about is when you open GraphQL-formatted text that came from somewhere else (e.g. it mixes full-width tabs with space-based indentation to effectively achieve 2-space indentation).

As a counter-point, the vim-elixir plugin doesn't set tabstop: https://github.com/elixir-editors/vim-elixir/blob/master/ftplugin/elixir.vim#L18

timfeirg commented 6 years ago

The case I'm thinking about is when you open GraphQL-formatted text that came from somewhere else (e.g. it mixes full-width tabs with space-based indentation to effectively achieve 2-space indentation).

Sorry I don't quite follow this, how can one use full-width (is that 8?) tabs and space-based (meaning using expandtab?) to achieve 2-space indentation?

I think it'd be awesome if I can use a 2-space indentation in my own project, but also respecting different indentation styles while editing .gql files composed elsewhere, but I don't think that can be achieved by solely setting these tab/indent options in vim.

I just got a vague impression that this is probably what you are talking about, if not please excuse my poor reading skills.

I think you should do it if you think that's the appropriate thing to do here, you have far more vimL experience than I, I only learned the meaning of softtabstop like yesterday

jparise commented 6 years ago

Here's an example of what I mean with sw=2 sts=2 ts=4 (when tabs haven't already been expanded):

{
··foo {
»···bar
··}
}

(· is a space character, and »··· is a tab character.)

I think I'm going to drop the tabstop setting. Let me know if you see any unexpected behavior after I do that.

mathstuf commented 6 years ago

I find it very annoying to have these set in the ftplugin file. There's no spec (unlike PEP8 for Python) and the rest of the project uses 4 spaces and this overwrites my indentation autodetector running. Can we just remove the indentation settings completely?

jparise commented 6 years ago

I don't feel very strongly about this. As noted above, it seems like most of the GraphQL community has elected to use 2-space indentation, but there is no formal standard.

... but if it's not generally seen as helpful to set this by default in this plugin, we can remove it. Folks could set it themselves in an after/ftplugin/graphql.vim file, although it's also true that folks who prefer a different style could do the same.

mathstuf commented 6 years ago

It's not that I prefer a different style for GraphQL in general, I'm just matching the rest of the project (which uses 4-space indentation everywhere). It's a per-project thing, not a per-filetype thing.

I just commented instead of making a PR since I'd rather there be some discussion.