{"thread":{"id":"61182","subject":"[PATCH] editorconfig: add Makefiles to \"text files\"","startedAt":"2024-03-22T22:18:28Z","lastAt":"2024-03-25T02:54:05Z","messageCount":6,"participants":["Max Gautier","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"491276","messageId":"20240322221813.13019-1-mg@max.gautier.name","threadId":"61182","inReplyTo":null,"subject":"[PATCH] editorconfig: add Makefiles to \"text files\"","fromName":"Max Gautier","fromEmail":"mg@max.gautier.name","sentAt":"2024-03-22T22:17:58Z","receivedAt":"2024-03-22T22:18:28Z","isPatch":true,"sender":{"key":"mg@max.gautier.name","avatar":"https://avatars.githubusercontent.com/u/13346812?v=4"},"body":"The Makefile and makefile fragments use the same indent style than the\nrest of the code (with some inconsistencies).\n\nAdd them to the relevant .editorconfig section to make life easier for\neditors and reviewers.\n\nSigned-off-by: Max Gautier <mg@max.gautier.name>\n---\n .editorconfig | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/.editorconfig b/.editorconfig\nindex f9d819623d..15d6cbeab1 100644\n--- a/.editorconfig\n+++ b/.editorconfig\n@@ -4,7 +4,7 @@ insert_final_newline = true\n \n # The settings for C (*.c and *.h) files are mirrored in .clang-format.  Keep\n # them in sync.\n-[*.{c,h,sh,perl,pl,pm,txt}]\n+[{*.{c,h,sh,perl,pl,pm,txt},config.mak.*,Makefile}]\n indent_style = tab\n tab_width = 8\n \n-- \n2.44.0\n\n"},{"id":"491281","messageId":"xmqqo7b5zy84.fsf@gitster.g","threadId":"61182","inReplyTo":"20240322221813.13019-1-mg@max.gautier.name","subject":"Re: [PATCH] editorconfig: add Makefiles to \"text files\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-22T22:40:59Z","receivedAt":"2024-03-22T22:41:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Gautier <mg@max.gautier.name> writes:\n\n> The Makefile and makefile fragments use the same indent style than the\n> rest of the code (with some inconsistencies).\n>\n> Add them to the relevant .editorconfig section to make life easier for\n> editors and reviewers.\n>\n> Signed-off-by: Max Gautier <mg@max.gautier.name>\n> ---\n>  .editorconfig | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/.editorconfig b/.editorconfig\n> index f9d819623d..15d6cbeab1 100644\n> --- a/.editorconfig\n> +++ b/.editorconfig\n> @@ -4,7 +4,7 @@ insert_final_newline = true\n>  \n>  # The settings for C (*.c and *.h) files are mirrored in .clang-format.  Keep\n>  # them in sync.\n> -[*.{c,h,sh,perl,pl,pm,txt}]\n> +[{*.{c,h,sh,perl,pl,pm,txt},config.mak.*,Makefile}]\n>  indent_style = tab\n>  tab_width = 8\n\nA question out of curiosity (because the answer does not affect any\nconclusion): Does editorconfig attempt to cover any non-text files?\n\nTwo more questions that do affect the conclusions are:\n\n * Among the files we ship (i.e. \"git ls-tree -r HEAD\") and edit\n   with editors that honor .editorconfig settings, are there any\n   file that we do not want tab indentation other than *.py?\n\n * Does .editorconfig file allow possibly conflicting setting, with\n   a reliable conflict resolution rules?\n\nWhat I am trying to get at is if it is possible to make something\nalong this line to work:\n\n    [*]\n\tcharset = utf-8\n\tinsert_final_newline = true\n\tindent_style = tab\n\ttab_width = 8\n    [*.py]\n\tindent_style = space\n\tindet_size = 4\n\nI am assuming, without knowing, that the conflict resolution rule\nmay be \"for the same setting, the last match wins\" so by default we\nalways use \"indent_style = tab\", but if we are talking about a Python\nscript, it is overruled with \"indent_style = space\".\n\nIf that is possible, we do not have to keep adding \"ah, files that\nmatch this pattern are also text\", i.e., everything is text and\nindented by tab, unless specified otherwise.\n\nThanks.\n"},{"id":"491300","messageId":"Zf77gyA28KsZdOUs@framework","threadId":"61182","inReplyTo":"xmqqo7b5zy84.fsf@gitster.g","subject":"Re: [PATCH] editorconfig: add Makefiles to \"text files\"","fromName":"Max Gautier","fromEmail":"mg@max.gautier.name","sentAt":"2024-03-23T15:55:47Z","receivedAt":"2024-03-23T15:58:09Z","isPatch":true,"sender":{"key":"mg@max.gautier.name","avatar":"https://avatars.githubusercontent.com/u/13346812?v=4"},"body":"On Fri, Mar 22, 2024 at 03:40:59PM -0700, Junio C Hamano wrote:\n> A question out of curiosity (because the answer does not affect any\n> conclusion): Does editorconfig attempt to cover any non-text files?\n\nApparently they it does not differentiate binary files:\nhttps://github.com/editorconfig/editorconfig-core-js/issues/42\nhttps://github.com/editorconfig/editorconfig/issues/285#issuecomment-267400370\n\n> Two more questions that do affect the conclusions are:\n> \n>  * Among the files we ship (i.e. \"git ls-tree -r HEAD\") and edit\n>    with editors that honor .editorconfig settings, are there any\n>    file that we do not want tab indentation other than *.py?\n\n$ git ls-tree -r HEAD | cut -f 2 | \\\n    grep -vE '.*\\.(c|h|sh|perl|pl|pm|txt)' | grep -v t/ \\\n    | rev | cut -d . -f -1 | rev | sort | uniq -c | grep -vE '\\<1\\>'\n-> gives for a first approximation (much more if not filtering unique\noccurrences)\n      2 bash\n      2 el\n      8 gitattributes\n     15 gitignore\n      4 go\n      2 in\n      7 js -> git-gui + git-web\n      8 md\n      2 png\n     41 po\n      2 pot\n     14 sample\n     41 tcl -> git-gui\n      5 xsl\n      5 yml -> ci stuff\nNot sure which one among those don't want the same tab-indent settings\nthough.\n\n\n> \n>  * Does .editorconfig file allow possibly conflicting setting, with\n>    a reliable conflict resolution rules?\n\nYeah it does: https://spec.editorconfig.org/#id8\nTL;DR:\n- from top to bottom, last matching section wins\n- if multiple .editorconfig are found (up until one with the root key or\n  in /) closest to the file wins.\n> \n> What I am trying to get at is if it is possible to make something\n> along this line to work:\n> \n>     [*]\n> \tcharset = utf-8\n> \tinsert_final_newline = true\n> \tindent_style = tab\n> \ttab_width = 8\n>     [*.py]\n> \tindent_style = space\n> \tindet_size = 4\n> \n> I am assuming, without knowing, that the conflict resolution rule\n> may be \"for the same setting, the last match wins\" so by default we\n> always use \"indent_style = tab\", but if we are talking about a Python\n> script, it is overruled with \"indent_style = space\".\n\nSo it looks like it's possible, if we also add judiciously .editorconfig\nin subdirectory where we have other files which don't want the same\nsettings, probably:\n- po/\n- t/\n- contrib/\n- .github/\n- ...\n\nNot sure if that's easier than adding stuff to the to the root config\nthough.\n\n-- \nMax Gautier\n"},{"id":"491305","messageId":"xmqqr0g0yhoe.fsf@gitster.g","threadId":"61182","inReplyTo":"Zf77gyA28KsZdOUs@framework","subject":"Re: [PATCH] editorconfig: add Makefiles to \"text files\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-23T17:36:01Z","receivedAt":"2024-03-23T17:36:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Gautier <mg@max.gautier.name> writes:\n\n>>  * Among the files we ship (i.e. \"git ls-tree -r HEAD\") and edit\n>>    with editors that honor .editorconfig settings, are there any\n>>    file that we do not want tab indentation other than *.py?\n>\n> $ git ls-tree -r HEAD | cut -f 2 | \\\n> ...\n>       2 png\n> ...\n\nAs I expected ;-) We do not have all that many kinds.  Some like\nimage files are not even meant to be edited as text and the usual\n\"editor\"s people use to edit them are probably unaware of what we\nwrite in the .editorconfig file.\n\n> Not sure which one among those don't want the same tab-indent settings\n> though.\n\nThat is the other important question.  What I was hoping to hear was\nthat other than Python there are not be any kind that are edited by\neditorconfig-aware editors.  But that unfortunately is not what we\nare hearing.\n\n>>  * Does .editorconfig file allow possibly conflicting setting, with\n>>    a reliable conflict resolution rules?\n>\n> Yeah it does: https://spec.editorconfig.org/#id8\n> TL;DR:\n> - from top to bottom, last matching section wins\n> - if multiple .editorconfig are found (up until one with the root key or\n>   in /) closest to the file wins.\n>> \n>> What I am trying to get at is if it is possible to make something\n>> along this line to work:\n>> \n>>     [*]\n>> \tcharset = utf-8\n>> \tinsert_final_newline = true\n>> \tindent_style = tab\n>> \ttab_width = 8\n>>     [*.py]\n>> \tindent_style = space\n>> \tindet_size = 4\n>> \n>> I am assuming, without knowing, that the conflict resolution rule\n>> may be \"for the same setting, the last match wins\" so by default we\n>> always use \"indent_style = tab\", but if we are talking about a Python\n>> script, it is overruled with \"indent_style = space\".\n>\n> So it looks like it's possible, if we also add judiciously .editorconfig\n> in subdirectory where we have other files which don't want the same\n> settings, probably:\n\nThat is much less than ideal---I was hoping that we can do this\nwith just one file.  My reading of that spec is that in the same\nfile it would be the last one wins, so something line what I gave\nyou above should work more-or-less as-is?\n\nAlso I am not sure if there is any reason why ...\n\n> - po/\n> - t/\n> - contrib/\n> - .github/\n> - ...\n>\n> Not sure if that's easier than adding stuff to the to the root config\n> though.\n\n... t/*.sh should use rules different from those that apply to\ncheck-builtins.sh at the root level, or contrib/mw-to-git/*.perl\nshould use Perl rules different from those that apply to\nperl/Git.pm.  So I think \"we need per-directory customization\" is a\nred herring.\n\nThe real potential downside of the approach to use a single default\nfallback set of rules with \"match everything\" is that types that we\ndid not tweak the editor rules for are suddenly and uniformly\nsubjected to a rule that may not match the contents.  We currently\ndo not do anything to .yml or .md so we do not force them to be in\nany consistent layout---even if all contributors use editorconfig\naware editor to edit them, they will produce inconsistent result.\nIf we enforce \"everybody indent with tab unless explicitly set\ndifferently like we do for .py\", these contributors consistently use\ntab indent on .yml or .md, which may not be a suitable convention\nfor the material.  My quick look says that .github/**/*.yml wants\ntwo-space indentation, and .md are fine with any indentation so\nenforcing tab indent consistently may be better than not enforcing\nany indent at all.\n\nSo, my gut feeling is that forcing \"everybody uses tab indent by\ndefault, and file types with specific needs should opt out just like\nwe do for Python\" may initially give us a bit of friction (e.g., We\nmay find .github/**/*.yml would want different rules, so we would\nadd \"[*.yml]\" section just like we have \"[*.py]\" section), but would\nmake the rule coverage more complete and more clear.  We would give\na .yml file some rule to follow, which may initially be wrong but we\ncan fix with explicit rule.\n\nSo I dunno.  If the Makefile snippet were the last type of files we\nwould ever add to .editorconfig, I think the patch under discussion\nis a good progress, but if we share the vision of \"more complete and\nclear rule coverage\", it is going in the opposite direction.\n\nLet me take your patch as-is, but leave it #leftoverbits to at least\nsee the feasibility of switching to \"everything falls under the\ndefault set of rules, and specific needs are dealt with exceptions\"\nmodel.\n\nThanks.\n\n"},{"id":"491342","messageId":"Zf_cSWY9DxVxKZu2@framework","threadId":"61182","inReplyTo":"xmqqr0g0yhoe.fsf@gitster.g","subject":"Re: [PATCH] editorconfig: add Makefiles to \"text files\"","fromName":"Max Gautier","fromEmail":"mg@max.gautier.name","sentAt":"2024-03-24T07:54:49Z","receivedAt":"2024-03-24T07:57:11Z","isPatch":true,"sender":{"key":"mg@max.gautier.name","avatar":"https://avatars.githubusercontent.com/u/13346812?v=4"},"body":"On Sat, Mar 23, 2024 at 10:36:01AM -0700, Junio C Hamano wrote:\n> >>  * Does .editorconfig file allow possibly conflicting setting, with\n> >>    a reliable conflict resolution rules?\n> >\n> > Yeah it does: https://spec.editorconfig.org/#id8\n> > TL;DR:\n> > - from top to bottom, last matching section wins\n> > - if multiple .editorconfig are found (up until one with the root key or\n> >   in /) closest to the file wins.\n> >> \n> >> What I am trying to get at is if it is possible to make something\n> >> along this line to work:\n> >> \n> >>     [*]\n> >> \tcharset = utf-8\n> >> \tinsert_final_newline = true\n> >> \tindent_style = tab\n> >> \ttab_width = 8\n> >>     [*.py]\n> >> \tindent_style = space\n> >> \tindet_size = 4\n> >> \n> >> I am assuming, without knowing, that the conflict resolution rule\n> >> may be \"for the same setting, the last match wins\" so by default we\n> >> always use \"indent_style = tab\", but if we are talking about a Python\n> >> script, it is overruled with \"indent_style = space\".\n> >\n> > So it looks like it's possible, if we also add judiciously .editorconfig\n> > in subdirectory where we have other files which don't want the same\n> > settings, probably:\n> \n> That is much less than ideal---I was hoping that we can do this\n> with just one file.  My reading of that spec is that in the same\n> file it would be the last one wins, so something line what I gave\n> you above should work more-or-less as-is?\n> \n\nI read it the same way, I didn't intend to imply using one top level\nonly was not possible ; sorry for the lack of clarity.\n\n> Also I am not sure if there is any reason why ...\n> \n> > - po/\n> > - t/\n> > - contrib/\n> > - .github/\n> > - ...\n> >\n> > Not sure if that's easier than adding stuff to the to the root config\n> > though.\n> \n> ... t/*.sh should use rules different from those that apply to\n> check-builtins.sh at the root level, or contrib/mw-to-git/*.perl\n> should use Perl rules different from those that apply to\n> perl/Git.pm.  So I think \"we need per-directory customization\" is a\n> red herring.\n\nOh, I was more thinking about the other stuff under t/ , not she scripts\nthemselves, there is some .test, .diff, lots of files without extensions\n(some css apparently, among other stuff) , and without looking in\ndetails, my best guess is that most of this is test samples (=> I mean\nthings used by the tests to compare / test processing result).\nI don't know if that's supposed to be edited manually though.\n\nBut yeah, \"per-directory customization\" isn't good, it multiplies the\nplace to look when the config is not correct. I was mentioning by fear\nthat using only one file would be hard to manage is there is too much\npatterns. \n\n-- \nMax Gautier\n"},{"id":"491390","messageId":"xmqqwmprm37b.fsf@gitster.g","threadId":"61182","inReplyTo":"Zf_cSWY9DxVxKZu2@framework","subject":"Re: [PATCH] editorconfig: add Makefiles to \"text files\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-25T02:54:00Z","receivedAt":"2024-03-25T02:54:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Gautier <mg@max.gautier.name> writes:\n\n> But yeah, \"per-directory customization\" isn't good, it multiplies the\n> place to look when the config is not correct. I was mentioning by fear\n> that using only one file would be hard to manage is there is too much\n> patterns. \n\nUnderstood.\n\nMy primary point is that we are not using too many patterns\ncurrently, and hence many files are *not* covered by .editorconfig\nrules.  Some files may not even be meant to be edited by an editor,\nbut many others are allowed to be edited without enforcing any style\nwith .editorconfig and how to format them is up to the developers.\nApplying a default rule to these files to enforce consistency may be\nbetter than uncontrolled random mess each developer may leave\nwithout any rules, and at the same time, doing so would mean we do\nnot have keep adding to the \"C uses this rule, Shell uses this rule,\noh, now we also want Makefile to use this use\" enumeration.\n\nThere may be fallouts (i.e., \"For files of type X that currently are\nnot governed by .editorconfig, the indent-with-tab rule is not\nsuitable, but suddenly these files are subject to the default rule\")\nbut adjusting the rules with exception would be more explicit and\nclarify the rules.\n\n"}]}