🏛 Proposal: RETAB EVERYTHING!

[pschmitt]

The more I work with zinit's core the more my eyes bleed.
Some lines are absurdly long.
A few examples:

• zinit/zinit-install.zsh

  Line 4 in 8b5ed2a

  builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || { builtin print -P "${ZINIT[col-error]}ERROR:%f%b Couldn't find ${ZINIT[col-obj]}zinit-side.zsh%f%b."; return 1; }

• zinit/zinit-install.zsh

  Line 995 in 8b5ed2a

  (( !OPTS[opt_-q,--quiet] )) && +zinit-message "Downloading {apo}\`{url}$sname{apo}\`{rst}${${ICE[svn]+" (with Subversion)"}:-" (with curl, wget, lftp)"}{…}"

• zinit/zinit-install.zsh

  Line 1848 in 8b5ed2a

  files=( ${(@)${(@s: :)${extract##(\!-|-\!|\!|-)}}//(#b)(((#s)|([^\\])[\\]([\\][\\])#)|((#s)|([^\\])([\\][\\])#)) /${match[2]:+$match[3]$match[4] }${match[5]:+$match[6]${(l:${#match[7]}/2::\\:):-} }} )

I get that most people don't stick to 80 chars for their code but I'd at least appreciate if we re-tabbed the zsh scripts to use 2 space chars instead of 4.

TL;DR
I'm advocating for vim: ft=zsh et ts=2 sw=2 for zinit*.zsh

[alichtman]

TL;DR: Your change is alright by me but doesn't completely solve the issue. We have to refactor the massive one-liners to really improve readability.

I agree that we have a readability problem to solve here. Personally, I like ts/sw=4, but I see the argument for using ts=2 here.

That said, I think the real fix is breaking up some of the massive one-liners. For example, the first link you included has 0 leading spaces but is still unruly to read and work with.

[pschmitt]

Some lines we cannot split easily without refactoring it all and possibly (probably) hurting performance. zsh expansion is powerful, but afaik there's no readable way to split these lines other than storing temporary results in a variable and doing the rest of the work in a second statement.

The best way forward here is to add a bunch of comments explaining the weirdest exps IMO. And unspaghetti what we encounter along the way. For instance I "fixed" the first example in #116:

zinit/zinit-install.zsh

Lines 4 to 7 in 7439160

	builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || {
	builtin print -P "${ZINIT[col-error]}ERROR:%f%b Couldn't find ${ZINIT[col-obj]}zinit-side.zsh%f%b."
	return 1
	}

Doing it all in one go would be a huge undertaking.

Another thing: to make things better I'm all for adopting a "return early" approach (I don't know the real tech term) ie:

Instead of:

fun() {
	  if check_is_okay {
	    long_block_of_code
	    return 0
	  } elif check_not_okay {
	    another_block_of_code
	    return 1
	  }
}

that:

fun() {
	  if check_not_okay {
	    another_block_of_code
	    return 1
	  }
	  # check is okay here
	  block_of_code
	  return 0
}

It's a poor example, but I'm sure you get my point.

Edit: It's a guard clause.

[alichtman]

Doing it all in one go

Yep, minor fixes over a long period of time is alright. Any improvements > no improvements :)

[vladdoster]

Closing in favor of #238.