arl / gitmux

:computer: Git in your tmux status bar
MIT License
630 stars 24 forks source link

feat: separate layout items with spaces #91

Closed joshmedeski closed 1 year ago

joshmedeski commented 1 year ago

Fixes #90

Purpose

Adds a space at the end of every layout component.

Approach

arl commented 1 year ago

Thank you @joshmedeski Can you add some unit test please?

joshmedeski commented 1 year ago

Sure thing!

joshmedeski commented 1 year ago

The way the tests are written make it hard to debug. I recommend moving away from a for loop for tests so when a test fails you get immediate feedback and a proper line number for what test failed.

arl commented 1 year ago

Each test in the for loop is a sub test. You can run a single sub test with go test -run TEST NAME/SUBTESTNAME

joshmedeski commented 1 year ago

I'm wondering if this should be a flag that doesn't cause a breaking change to the way everything is rendered now? (like tailing_spaces?)

Let me know your thoughts.

arl commented 1 year ago

I'm wondering if this should be a flag that doesn't cause a breaking change to the way everything is rendered now? (like tailing_spaces?)

Let me know your thoughts.

Yep this might be needed if the change is too important. Let me think about it

joshmedeski commented 1 year ago

I got the tests working and I got the feature working to the best of my ability for now.

But if someone doesn't like having the trailing spaces between each layout component we'll have to write more code to handle that.

arl commented 1 year ago

Adds a space at the end of every layout component. @joshmedeski

What's the difference between adding spaces manually in the layout section?

For example, this is the default layout section:

layout: [branch, remote-branch, divergence, "- ", flags]

This is the one I've been using:

layout: [branch, ' ', remote-branch, divergence, ' - ', flags, ' ', stats]
joshmedeski commented 1 year ago

So in your second example, when divergence, flags, and / or stats have nothing to show. You end up with extra white spaces in the tmux status bar (see the screenshots from the issue). I'd rather trim the whitespace if there's nothing to render.

arl commented 1 year ago

@joshmedeski I've tagged a new release with both your pull-requests (v0.10.0) Thanks again for the help!

joshmedeski commented 1 year ago

That makes more sense. This was my first time in the codebase so I'm starting to get my way around.

I had the same idea later on to move this logic in the place that chains the layout components together, thanks for doing it!

arl commented 1 year ago

Sure, no worries. You did a great job 🎉

joshmedeski commented 1 year ago

Can we update to make sure the final component includes a space if anything is rendered?

Screenshot 2023-03-22 at 9 54 39 AM
arl commented 1 year ago

Can we update to make sure the final component includes a space if anything is rendered?

Screenshot 2023-03-22 at 9 54 39 AM

I don't understand. Do you mean a trailing space?

joshmedeski commented 1 year ago

Exactly

arl commented 1 year ago

Ah i think I got it. I think you mean a trailing space, in case anything was rendered. That's a particular case though. Majority of people, myself included, do not have anything after gitmux status shown on their tmux status bar. Isn't that something you can fix by adding a trailing space to your config layout?

joshmedeski commented 1 year ago

Fair point, so my tmux status is {session_name} {gitmux} {windows}. But sometimes I load tmux sessions that don't have a gitmux session so I'm hoping to have {session_name} {windows} render without any extra spaces.

It's a little tricky. Maybe I can add a trailing_space option to support this situation? Just adding a space at the end of my layout potentially creates two spaces when I was hoping for just one on tmux sessions that don't render anything in gitmux.

Hope all that makes sense.

arl commented 1 year ago

That seems a little specific to your use case to be honest

I think you might be able to do that with the help of a simple script, called instead of gitmux, from your tmux.conf, and depending on whether gitmux string is empty, you add a trailing space or not. With some tmux status string ninja-foo, you might even be able to do that without the help of an addition script.

It's not that I'm against that feature or anything, but I need to set the bar for features everybody can benefit from. sorry. If you're willing to open an issue for that, and let see if there are other users with that need.

joshmedeski commented 1 year ago

I implemented the feature, see #94

This is a non-breaking change but is very useful for people who want to place gitmux between other elements in their tmux status bar. Hopefully you find it not too invasive and decide to merge it.