git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Add an EditorConfig file
@ 2018-09-17 23:03 brian m. carlson
  2018-09-17 23:18 ` Taylor Blau
  2018-09-19  3:00 ` Junio C Hamano
  0 siblings, 2 replies; 12+ messages in thread
From: brian m. carlson @ 2018-09-17 23:03 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau

Contributors to Git use a variety of editors, each with their own
configuration files.  Because C lacks the defined norms on how to indent
and style code that other languages, such as Ruby and Rust, have, it's
possible for various contributors, especially new ones, to have
configured their editor to use a style other than the style the Git
community prefers.

To make automatically configuring one's editor easier, provide an
EditorConfig file.  This is an INI-style configuration file that can be
used to specify editor settings and can be understood by a wide variety
of editors.  Some editors include this support natively; others require
a plugin.  Regardless, providing such a file allows users to
automatically configure their editor of choice with the correct settings
by default.

Provide global settings to set the character set to UTF-8 and insert a
final newline into files.  Provide language-specific settings for C,
Shell, Perl, and Python files according to what CodingGuidelines already
specifies.  Since the indentation of other files varies, especially
certain AsciiDoc files, don't provide any settings for them until a
clear consensus forward emerges.

Don't specify an end of line type.  While the Git community uses
Unix-style line endings in the repository, some Windows users may use
Git's auto-conversion support and forcing Unix-style line endings might
cause problems for those users.

Finally, leave out a root directive, which would prevent reading other
EditorConfig files higher up in the tree, in case someone wants to set
the end of line type for their system in such a file.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
I was incentivized to write this when I started using worktrees for
development and found that I had inserted spaces into all the work I was
doing because I was outside of my normal git.git clone.

This is the easiest way to set per-repo configuration across editors,
especially since people may work on various C codebases with different
indentation standards.

 .editorconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index 0000000000..8963d83fdb
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,11 @@
+[*]
+charset = utf-8
+insert_final_newline = true
+
+[*.{c,h,sh,perl}]
+indent_style = tab
+tab_width = 8
+
+[*.py]
+indent_style = space
+indent_size = 4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-17 23:03 [PATCH] Add an EditorConfig file brian m. carlson
@ 2018-09-17 23:18 ` Taylor Blau
  2018-09-20  0:05   ` brian m. carlson
  2018-09-19  3:00 ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Taylor Blau @ 2018-09-17 23:18 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Taylor Blau

Hi brian,

Thanks for CC-ing me on this.

I use editorconfig every day via the configuration in my home directory
[1], and the Vim plugin editorconfig-vim [2]. It's a great piece of
software, and I've been using it without any issue since around the
beginning of 2015.

On Mon, Sep 17, 2018 at 11:03:07PM +0000, brian m. carlson wrote:
> Regardless, providing such a file allows users to automatically
> configure their editor of choice with the correct settings by default.

I think that this is the central argument to be made here for keeping
this out of contrib/, and in the root tree. Most editor (all?) plugins
will pick this location up automatically, which ought to cut down on
patches that aren't formatted correctly.

> Provide global settings to set the character set to UTF-8 and insert a
> final newline into files.  Provide language-specific settings for C,
> Shell, Perl, and Python files according to what CodingGuidelines already
> specifies.  Since the indentation of other files varies, especially
> certain AsciiDoc files, don't provide any settings for them until a
> clear consensus forward emerges.
>
> Don't specify an end of line type.  While the Git community uses
> Unix-style line endings in the repository, some Windows users may use
> Git's auto-conversion support and forcing Unix-style line endings might
> cause problems for those users.

Good. Even the official editorconfig documentation specifies that this
ought to be the responsibility "of the VCS" [3], a point on which I
agree.

> +[*.{c,h,sh,perl}]
> +indent_style = tab
> +tab_width = 8

In all *.[ch] files in git.git, I found a total of 817 lines over 79
characters wide:

  $ for file in $(git ls-files '**/*.[ch]'); do
      awk 'length > 79' $f;
    done | wc -l

So I think that specifying indent_style, and tab_width to be 'tab' and
'8' respectively is enough. We shouldn't be enforcing a rule about line
lengths that we are not ourselves consistently following.

Have you thought about including guidelines on COMMIT_EDITMSG? We prefer
72 characters per line [3], and this is enforceable via the following:

  [COMMIT_EDITMSG]
  max_line_length = 72

Thanks,
Taylor

[1]: https://github.com/ttaylorr/dotfiles/blob/work-gh/editorconfig/.editorconfig
[2]: https://github.com/editorconfig/editorconfig-vim
[3]: http://public-inbox.org/git/20170930070127.xvtn7dfyuoh26mhp@sigill.intra.peff.net

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-17 23:03 [PATCH] Add an EditorConfig file brian m. carlson
  2018-09-17 23:18 ` Taylor Blau
@ 2018-09-19  3:00 ` Junio C Hamano
  2018-09-20  0:00   ` brian m. carlson
  1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2018-09-19  3:00 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Taylor Blau

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> To make automatically configuring one's editor easier, provide an
> EditorConfig file.  This is an INI-style configuration file that can be
> used to specify editor settings and can be understood by a wide variety
> of editors.  Some editors include this support natively; others require
> a plugin.

Good intentions.  One thing that makes me wonder is how well this
plays with "make style" and how easy will it be to keep their
recommendations in sync.  Ideally, if we can generate one from the
other, or both from a unified source, that would be great.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-19  3:00 ` Junio C Hamano
@ 2018-09-20  0:00   ` brian m. carlson
  2018-09-20 14:08     ` Junio C Hamano
  2018-09-21  2:26     ` Eric Sunshine
  0 siblings, 2 replies; 12+ messages in thread
From: brian m. carlson @ 2018-09-20  0:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Taylor Blau

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

On Tue, Sep 18, 2018 at 08:00:27PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > To make automatically configuring one's editor easier, provide an
> > EditorConfig file.  This is an INI-style configuration file that can be
> > used to specify editor settings and can be understood by a wide variety
> > of editors.  Some editors include this support natively; others require
> > a plugin.
> 
> Good intentions.  One thing that makes me wonder is how well this
> plays with "make style" and how easy will it be to keep their
> recommendations in sync.  Ideally, if we can generate one from the
> other, or both from a unified source, that would be great.

I think "make style" and the EditorConfig file are complementary.  "make
style" autoformats code into a diff.  I agree that if we always used
clang-format to format code, then this would be a non-issue in the
EditorConfig file, since we'd just tell people to format their code and
be done with it.  However, we don't automatically do that, so I think
this still has value.

(I am having trouble getting make style to work, though, because it
seems to invoke clang-format as a git subcommand, and I don't think that
works.  I may send a patch.)

Even if we did that, we couldn't do it for Perl, because the tidy tool
for Perl, perltidy, produces different output depending on version.  I
expect we'll have little success in trying to standardize on a given
version.

The EditorConfig file applies to a variety of formats and is designed to
set default editor settings (which users *can* override if they choose).
It applies to any file pattern; for example, as Taylor pointed out, we
could use it to recommend 72-character lines in commit messages.

I agree that maintaining more files is a hassle, but personally, I think
it quite unlikely that we're going to change the Git style, so I feel
the maintenance is fairly low.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-17 23:18 ` Taylor Blau
@ 2018-09-20  0:05   ` brian m. carlson
  0 siblings, 0 replies; 12+ messages in thread
From: brian m. carlson @ 2018-09-20  0:05 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

On Mon, Sep 17, 2018 at 07:18:50PM -0400, Taylor Blau wrote:
> Hi brian,
> 
> Thanks for CC-ing me on this.
> 
> I use editorconfig every day via the configuration in my home directory
> [1], and the Vim plugin editorconfig-vim [2]. It's a great piece of
> software, and I've been using it without any issue since around the
> beginning of 2015.

Yeah, I noticed it in your dotfiles.

> In all *.[ch] files in git.git, I found a total of 817 lines over 79
> characters wide:
> 
>   $ for file in $(git ls-files '**/*.[ch]'); do
>       awk 'length > 79' $f;
>     done | wc -l
> 
> So I think that specifying indent_style, and tab_width to be 'tab' and
> '8' respectively is enough. We shouldn't be enforcing a rule about line
> lengths that we are not ourselves consistently following.

I must admit that some of these are my fault.  I think we're moving to
use clang-format more as a tool, so hopefully those become less of an
issue over time.

> Have you thought about including guidelines on COMMIT_EDITMSG? We prefer
> 72 characters per line [3], and this is enforceable via the following:
> 
>   [COMMIT_EDITMSG]
>   max_line_length = 72

That's an interesting thought.  I know different people have different
settings for this: 70, 72, 74.  72 characters is typical for emails, and
since our commits end up as emails, I think standardizing on 72 would
make sense.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-20  0:00   ` brian m. carlson
@ 2018-09-20 14:08     ` Junio C Hamano
  2018-09-21 22:50       ` brian m. carlson
  2018-09-21  2:26     ` Eric Sunshine
  1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2018-09-20 14:08 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Taylor Blau

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I think "make style" and the EditorConfig file are complementary.  "make
> style" autoformats code into a diff.  I agree that if we always used
> clang-format to format code, then this would be a non-issue in the
> EditorConfig file, since we'd just tell people to format their code and
> be done with it.  However, we don't automatically do that, so I think
> this still has value.

Oh, we agree on that 100%.  These are complementary.  

My comment was that it would be confusing if they gave contradicting
suggestions to the end user.  After letting EditorConfig to enforce
one style while typing and saving, if "make style" suggests to
format it differently, it would not be a great user experience.

The ideal response would have been "Oh, of course EditorConfig folks
already thought about that, which is a natural thing to wish for,
and they have a tool to generate clang-format configuration from the
section for C language in any EditorConfig file---here is a link.
After all, tools like clang-format look like just another editor to
them ;-)".

But that is a response in a dream-world.  If there is no such tool,
I am perfectly OK if the plan is to manually keep them (loosely) in
sync.  I do not think it is good use of our time to try to come up
with such a tool (unless somebody is really interested in doing it,
that is).

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-20  0:00   ` brian m. carlson
  2018-09-20 14:08     ` Junio C Hamano
@ 2018-09-21  2:26     ` Eric Sunshine
  2018-09-21  2:35       ` Jeff King
  1 sibling, 1 reply; 12+ messages in thread
From: Eric Sunshine @ 2018-09-21  2:26 UTC (permalink / raw)
  To: brian m. carlson, Junio C Hamano, Git List, Taylor Blau

On Wed, Sep 19, 2018 at 8:00 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> (I am having trouble getting make style to work, though, because it
> seems to invoke clang-format as a git subcommand, and I don't think that
> works.  I may send a patch.)

You're probably missing this piece:
https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-21  2:26     ` Eric Sunshine
@ 2018-09-21  2:35       ` Jeff King
  2018-09-21 22:19         ` brian m. carlson
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff King @ 2018-09-21  2:35 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: brian m. carlson, Junio C Hamano, Git List, Taylor Blau

On Thu, Sep 20, 2018 at 10:26:47PM -0400, Eric Sunshine wrote:

> On Wed, Sep 19, 2018 at 8:00 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > (I am having trouble getting make style to work, though, because it
> > seems to invoke clang-format as a git subcommand, and I don't think that
> > works.  I may send a patch.)
> 
> You're probably missing this piece:
> https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format

I've been confused by this, too. In Debian they ship it with the version
number. E.g., /usr/bin/git-clang-format-7, which is a symlink to
/usr/lib/llvm-7/bin/git-clang-format.

We might want a Makefile variable to let you override this path (I think
I just ended up putting another symlink in ~/local/bin).

-Peff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-21  2:35       ` Jeff King
@ 2018-09-21 22:19         ` brian m. carlson
  0 siblings, 0 replies; 12+ messages in thread
From: brian m. carlson @ 2018-09-21 22:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Sunshine, Junio C Hamano, Git List, Taylor Blau

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

On Thu, Sep 20, 2018 at 10:35:04PM -0400, Jeff King wrote:
> On Thu, Sep 20, 2018 at 10:26:47PM -0400, Eric Sunshine wrote:
> 
> > On Wed, Sep 19, 2018 at 8:00 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > > (I am having trouble getting make style to work, though, because it
> > > seems to invoke clang-format as a git subcommand, and I don't think that
> > > works.  I may send a patch.)
> > 
> > You're probably missing this piece:
> > https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format

I am indeed missing that.  I looked for it in git.git, but didn't think
to look for it as part of clang-format.

> I've been confused by this, too. In Debian they ship it with the version
> number. E.g., /usr/bin/git-clang-format-7, which is a symlink to
> /usr/lib/llvm-7/bin/git-clang-format.

I filed a bug report asking Debian to include a symlink in the default
package.

> We might want a Makefile variable to let you override this path (I think
> I just ended up putting another symlink in ~/local/bin).

I may send a patch to do that.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-20 14:08     ` Junio C Hamano
@ 2018-09-21 22:50       ` brian m. carlson
  2018-09-24 20:05         ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: brian m. carlson @ 2018-09-21 22:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Taylor Blau

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On Thu, Sep 20, 2018 at 07:08:35AM -0700, Junio C Hamano wrote:
> My comment was that it would be confusing if they gave contradicting
> suggestions to the end user.  After letting EditorConfig to enforce
> one style while typing and saving, if "make style" suggests to
> format it differently, it would not be a great user experience.

Ah, okay.  I understand your concern better now.

> The ideal response would have been "Oh, of course EditorConfig folks
> already thought about that, which is a natural thing to wish for,
> and they have a tool to generate clang-format configuration from the
> section for C language in any EditorConfig file---here is a link.
> After all, tools like clang-format look like just another editor to
> them ;-)".

No, I don't think there's such a tool.  clang-format wants a lot of
format settings that aren't in EditorConfig, and EditorConfig wants to
address things that are not C source files.

> But that is a response in a dream-world.  If there is no such tool,
> I am perfectly OK if the plan is to manually keep them (loosely) in
> sync.  I do not think it is good use of our time to try to come up
> with such a tool (unless somebody is really interested in doing it,
> that is).

Would it be helpful if I sent a script that ran during CI to ensure they
stayed in sync for the couple places where they overlap?  I'm happy to
do so if you think it would be useful.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-21 22:50       ` brian m. carlson
@ 2018-09-24 20:05         ` Junio C Hamano
  2018-09-27 22:33           ` brian m. carlson
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2018-09-24 20:05 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Taylor Blau

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>> But that is a response in a dream-world.  If there is no such tool,
>> I am perfectly OK if the plan is to manually keep them (loosely) in
>> sync.  I do not think it is good use of our time to try to come up
>> with such a tool (unless somebody is really interested in doing it,
>> that is).
>
> Would it be helpful if I sent a script that ran during CI to ensure they
> stayed in sync for the couple places where they overlap?  I'm happy to
> do so if you think it would be useful.

It may even be an overkill.

A comment addressed to those who edit one file to look at the other
file on both files would be sufficient, I suspect.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add an EditorConfig file
  2018-09-24 20:05         ` Junio C Hamano
@ 2018-09-27 22:33           ` brian m. carlson
  0 siblings, 0 replies; 12+ messages in thread
From: brian m. carlson @ 2018-09-27 22:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Taylor Blau

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On Mon, Sep 24, 2018 at 01:05:23PM -0700, Junio C Hamano wrote:
> > Would it be helpful if I sent a script that ran during CI to ensure they
> > stayed in sync for the couple places where they overlap?  I'm happy to
> > do so if you think it would be useful.
> 
> It may even be an overkill.
> 
> A comment addressed to those who edit one file to look at the other
> file on both files would be sufficient, I suspect.

Sure.  Let me reroll with that change.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-09-27 22:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 23:03 [PATCH] Add an EditorConfig file brian m. carlson
2018-09-17 23:18 ` Taylor Blau
2018-09-20  0:05   ` brian m. carlson
2018-09-19  3:00 ` Junio C Hamano
2018-09-20  0:00   ` brian m. carlson
2018-09-20 14:08     ` Junio C Hamano
2018-09-21 22:50       ` brian m. carlson
2018-09-24 20:05         ` Junio C Hamano
2018-09-27 22:33           ` brian m. carlson
2018-09-21  2:26     ` Eric Sunshine
2018-09-21  2:35       ` Jeff King
2018-09-21 22:19         ` brian m. carlson

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).