This seems fine. Confirmed that the changes are only whitespace.

I think some folks prefer tabs in javascript for character count reasons, but in a world where pages routinely load megs and megs of js files it seems like a useless optimization. Better to use spaces as you suggest.

We could minimize it in Makefile, but as you say - it is almost irrelevant there days.

I'm fine with tab indents, as long as they're used consistently — meaning, 1 tab per indent level. None of this "4-column indents, but every 2 levels is a tab" nonsense, that always breaks. If people want to use tabs and also want indents to be 4 columns, they can set their tab width to 4.

And, it's true that the spaces increase the file size. It's not a very big file, but the existing file is 5316 bytes. Converting to space indents (this PR) blows that up to 5988 bytes. Still not big, but quite a bit bigger.

An all-tabs-indented version (on my format-js-2 branch) that uses spaces only for alignment on wrapped calls (one 2-line if and the remaining arguments to one long req.open()) is only 5088 bytes. I can open a PR with that change, instead, if you prefer?

I'd stick with spaces. The tabs were almost certainly there by accident

I added a second commit creating a preliminary .editorconfig in the repo root, formalizing some of these rules.

So far it only has:

1. All files are UTF-8, LF endings, no trailing whitespace, final newline
2. *.js and *.py use 4-space indents
3. Makefile uses 8-column tab indents

I can add more rules for other file types, if anyone has suggestions?

Hmm. Are the .sql files in schemas/ generated, or hand-coded? Because, looking at schema.sql, seems it's also besieged by tab- and space-indented anarchy.

(At least the indentation's consistent: 8 columns. It just can't decide how it wants to get from column 1 to column 8.)

Just added *.css and *.sh to the 4-space indent section (matches format of existing files), and listed some extensionless Python files (runtests, cli/koji, vm/kojivmd) explicitly so the rules will apply to them as well.

Still a bit more:

$ rg -l \\t |grep -v Makefile|grep -v schema|grep -v watchlogs
LGPL
devtools/kojipolicy.vim
util/kojira.init
www/kojiweb/externalrepoinfo.chtml
www/kojiweb/taginfo.chtml
www/kojiweb/packages.chtml
vm/kojivmd.init
tests/test_lib/data/specs/test-files._spec
tests/test_lib/data/specs/test-src._spec
tests/test_lib/data/specs/test-deps._spec

Thanks, @tkopecek

LGPL

Definitely can't touch that. Learned my lesson there.

devtools/kojipolicy.vim
tests/test_lib/data/specs/test-files._spec
tests/test_lib/data/specs/test-src._spec
tests/test_lib/data/specs/test-deps._spec

These tabs all look intentional. I'm reluctant to mess with them.

util/kojira.init
vm/kojivmd.init

Heh. There was one tab in each file. Fixing.

www/kojiweb/externalrepoinfo.chtml
www/kojiweb/taginfo.chtml
www/kojiweb/packages.chtml

Yeah, these are also a mess of mixed indentation. Will try to sort them out. And it looks like .chtml should use 2-space indents, will add to .editorconfig.

Coming back to the schema files, only 7 of 35 contain any tabs, and mostly not too many — except for the main schema.sql which is mostly tabs.

$ grep -Ec '^ ' schema.sql
143
$ grep -c '     ' *.sql|grep -v ':0$'
schema.sql:435
schema-upgrade-1.17-1.18.sql:3
schema-upgrade-1.18-1.19.sql:13
schema-upgrade-1.21-1.22.sql:1
schema-upgrade-1.2-1.3.sql:24
schema-upgrade-1.9-1.10.sql:13

I was also wrong about the consistency in how they're used. There are (based on the surrounding space-indented code) sections formatted for 4-column tabs, sections formatted for 8-column tabs, lines with 8-space indents, lines with 4-space indents, and this one procedure (schema.sql lines 813-824) that uses a 3-space initial indent, then 4-space indents after that:

-- index for default search method for rpms, PG11+ can benefit from new include method
DO $$
   DECLARE version integer;
   BEGIN
       SELECT current_setting('server_version_num')::integer INTO version;
       IF version >= 110000 THEN
           EXECUTE 'CREATE INDEX rpminfo_filename ON rpminfo((name || ''-'' || version || ''-'' || release || ''.'' || arch || ''.rpm'')) INCLUDE (id);';
       ELSE
           EXECUTE 'CREATE INDEX rpminfo_filename ON rpminfo((name || ''-'' || version || ''-'' || release || ''.'' || arch || ''.rpm''));';
       END IF;
   END
$$;

I could standardize all of that (though the question remains: around what rules?), but it's going to mean major reformatting to the main schema.sql in particular.

And even the files without tabs aren't entirely consistent. schema-upgrade-1.34-1.35.sql, for example, uses 8-space indents... except for the last line, which has a 4-space indent.

Maybe it's best to just leave these alone?

devtools/kojipolicy.vim
tests/test_lib/data/specs/test-files._spec
tests/test_lib/data/specs/test-src._spec
tests/test_lib/data/specs/test-deps._spec

These tabs all look intentional. I'm reluctant to mess with them.

More to the point, they're all inner tabs, not indentation.

(re: schemas/*.sql)

Maybe it's best to just leave these alone?

If so, then this PR is complete. I've addressed every tab-containing file except the schemas/ dir and the "untouchables" (the license file and the ones listed above).

If I should reformat the schemas/ files, then someone please define the set of indentation rules they should follow, as the existing formatting is inconsistent enough that there's no one obvious choice.

@mikem @tkopecek Anything else you need from me on this?

issue #4187

I would leave it as it is now. Thanks! (scheduled for next minor release)

lgtm

Commit dc3eeded fixes this pull-request