Add spinner config options

[belyakov-am]

• PR Description

Add spinner config section to the user config. The section has 2 options:

• frames - list of strings that are iterated in the spinner animation
• rate - spinner's rate in milliseconds

Closes #3435

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go generate ./...)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• Docs (specifically docs/Config.md) have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[stefanhaller]

Very nice, good work. Several thoughts:

• It would be more flexible to make it an array of strings; this would allow users to set it to something like loaderFrames: [o.., .o., ..o, .o.] if they want to.
• For many animation patterns, our default framerate of 50ms is way too fast. For the above example, something like 120ms looks better. I wonder if we should make this configurable as well. (If you do, note that it needs to be used in two places, in Loader and in renderAppStatusSync.)
• We use the term "Loader" in the code, but I'm not sure it's the best user-facing term to use. We use the animation in many cases where nothing is loaded, e.g. when pushing a branch, or executing a custom command. In other contexts, the term "spinner" is used for this kind of thing, maybe that's better.

Putting it all together, I could imagine something like

gui:
  spinner:
    frames: ['|', '/', '-', '\']
    rate: 50

And I would definitely want to hear @jesseduffield's opinion on all this before I merge anything.

[belyakov-am]

Thanks for your comment! All of your points make sense, I will push the necessary updates.

And yes, let's see what Jesse thinks about these changes.

[jesseduffield]

I agree with all of Stefan's feedback :)

[belyakov-am]

@stefanhaller kindly ask to review

[jesseduffield]

LGTM, I tested with:

gui:
  spinner:
      frames: ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']
      rate: 100

Also tested with a longer spinner so to ensure nothing broke.

Great work!

[jesseduffield]

@stefanhaller let me know if you wanna take a final look at this before we merge

[jesseduffield]

Also @belyakov-am this is a small enough PR that I reckon you should just squash all your commits into one

[stefanhaller]

Yes, I wanted to review this again, I think there are one or two smaller things that can be improved still. Didn't have time for that in the last few days. (Yes, happens to me, too 😅)

Agree with the squashing.

[belyakov-am]

Thanks!

Sure, I'll squash before merging

I would prefer not to rush with merging and make updates after Stefan's review first

[stefanhaller]

I pushed one commit that simplifies the code a little bit, and I have two very minor nitpicks below. Apart from that, I'm very happy with this, great work.

As for the squashing: I prefer to review PRs commit by commit, which is basically impossible here; I had to review the "All changes" tab. Next time, it would be much appreciated if you would try to structure your commits up front so that they make sense as a series of logical changes.

[stefanhaller]

The # spinner frames comment adds zero new information, I'd either drop it, or rephrase it to something like # the frames of the spinner animation. Dropping would be fine for me.

[stefanhaller]

I'd say // The frames of the spinner animation. It's not necessarily characters.

[belyakov-am]

Thanks for a thorough review! I'll take a note regarding commits structure for further PRs.