Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix format seperator #343

Merged
merged 4 commits into from
Feb 26, 2022
Merged

Conversation

asyncmind0
Copy link

@asyncmind0 asyncmind0 commented Oct 31, 2021

Rebase of

#305

@tony
Copy link
Member

tony commented Oct 31, 2021

@jagguli Thank you!

@tony
Copy link
Member

tony commented Oct 31, 2021

@jagguli If you want a smoother rebase, you can do git pull --rebase origin master and git push --force-with-lease where origin is https://github.com/tmux-python/libtmux.git

@tmux-python tmux-python deleted a comment from codecov bot Oct 31, 2021
@CLAassistant
Copy link

CLAassistant commented Jan 11, 2022

CLA assistant check
All committers have signed the CLA.

@tony
Copy link
Member

tony commented Feb 22, 2022

@jagguli Sorry for the delay! You are welcome to do another rebase

Also does this have a dedicated bug issue documenting what the problem is? (This is a pretty systemic change!)

@@ -7,6 +7,11 @@

"""

from __future__ import absolute_import, unicode_literals, with_statement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's safe to remove this line

@tony tony self-requested a review February 22, 2022 16:05
Copy link
Member

@tony tony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#354 is on deck and may cause conflicts. Would like to get this in first if possible. Some minor comments made

@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #343 (666e4d2) into master (e7c53ca) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 666e4d2 differs from pull request most recent head 9411cc1. Consider uploading reports for the commit 9411cc1 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #343   +/-   ##
=======================================
  Coverage   87.80%   87.81%           
=======================================
  Files          15       15           
  Lines        1509     1510    +1     
=======================================
+ Hits         1325     1326    +1     
  Misses        184      184           
Impacted Files Coverage Δ
libtmux/formats.py 100.00% <100.00%> (ø)
libtmux/server.py 79.19% <100.00%> (-0.12%) ⬇️
libtmux/session.py 81.54% <100.00%> (ø)
libtmux/window.py 84.33% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5458b98...9411cc1. Read the comment docs.

@tony tony force-pushed the fix_format_seperator branch 4 times, most recently from a077881 to 9c1fe18 Compare February 26, 2022 16:29
tony pushed a commit that referenced this pull request Feb 26, 2022
@tony tony force-pushed the fix_format_seperator branch from 9c1fe18 to 9f67200 Compare February 26, 2022 16:43
asyncmind0 and others added 4 commits February 26, 2022 10:46
- for some reason on my arch install tmux does not print back tabs

I'm not sure if it affects other but just puttin this out there
@tony tony force-pushed the fix_format_seperator branch from 9f67200 to 9411cc1 Compare February 26, 2022 16:46
@tony tony merged commit b231346 into tmux-python:master Feb 26, 2022
@@ -6,6 +6,9 @@
For reference: https://github.com/tmux/tmux/blob/master/format.c

"""
import os

FORMAT_SEPERATOR = os.environ.get("TMUX_SEPERATOR", "|")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astonishingly, despite "seperator" being used everywhere (and it feels right to me) apparently its separator

I will make a change for that and also give it a namespaced env name LIBTMUX_TMUX_FORMAT_SEPARATOR to avoid any ambiguity

Copy link
Member

@tony tony Feb 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

24393a3, 76faeb1

(side notes: FYI these refs are subject to break pending change #350)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. I opened #355 to discuss a default formatting string

tony added a commit that referenced this pull request Feb 26, 2022
tony added a commit that referenced this pull request Feb 26, 2022
Introduced initially via #343 as TMUX_SEPERATOR but later renamed
to LIBTMUX_TMUX_FORMAT_SEPARATOR.
tony added a commit that referenced this pull request Feb 26, 2022
tony added a commit that referenced this pull request Feb 26, 2022
Introduced initially via #343 as TMUX_SEPERATOR but later renamed
to LIBTMUX_TMUX_FORMAT_SEPARATOR.
@tony
Copy link
Member

tony commented Feb 26, 2022

@jagguli This is now released on 0.11.0b0 for testing: pypi, git branch

The separator is configurable via LIBTMUX_TMUX_FORMAT_SEPRATOR. A discussion is at #355 for any issues the arise

@tony
Copy link
Member

tony commented Feb 26, 2022

@jagguli Similar to the other PR, my bad for now staying on top of the pull request. I apologize for keeping it in a holding pattern so long

Shoot me an email next time you make a PR so it doesn't repeat itself. I am overwhelmed with notifications on GH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants