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

Add --log-file option to load command #647

Merged
merged 7 commits into from Nov 22, 2020
Merged

Add --log-file option to load command #647

merged 7 commits into from Nov 22, 2020

Conversation

joseph-flinn
Copy link
Contributor

Adding the --log-file option to the load command.

(As discussed over email. There isn't a specific issue for the addition)

@codecov
Copy link

codecov bot commented Nov 15, 2020

Codecov Report

Merging #647 (287fcc1) into v1.6.x (8858179) will increase coverage by 0.36%.
The diff coverage is 46.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           v1.6.x     #647      +/-   ##
==========================================
+ Coverage   73.83%   74.19%   +0.36%     
==========================================
  Files           7        7              
  Lines        1055     1062       +7     
  Branches      269      271       +2     
==========================================
+ Hits          779      788       +9     
+ Misses        202      200       -2     
  Partials       74       74              
Impacted Files Coverage Δ
tmuxp/cli.py 71.60% <46.42%> (+1.01%) ⬆️

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 8858179...287fcc1. Read the comment docs.

@joseph-flinn joseph-flinn marked this pull request as ready for review November 15, 2020 19:21
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.

Nice @joseph-flinn !

The logger does such a good job it preserves colors:

tmuxp load ~/work/python/libtmux --log-file ~/mylog.log
image

In another tab:

cat mylog.log
image

~ ❯ cat mylog.log
(INFO) [15:11:15] tmuxp.cli  [Loading] /home/t/work/python/libtmux/.tmuxp.yaml
  1. Should we add an ability to control log level? I don't think one is available is it?
  2. Should we add an option to stripe formatting?

cat looks beautiful, but for the text itself in an editor:
image

^[[0m^[[32m^[[1m(INFO)^[[0m [^[[30m^[[2m^[[1m15:11:15^[[39m^[[0m] ^[[37m^[[2m^[[1mtmuxp.cli^[[39m^[[0m ^[[0m [Loading] /home/t/work/python/libtmux/.tmuxp.yaml

@joseph-flinn
Copy link
Contributor Author

joseph-flinn commented Nov 15, 2020

  1. I think there is a way to set the log level from the click.group by including --log_level <LEVEL> before the load command. This should be passed through to tmuxp_echo. I did just notice that --log_level uses underscores (left over from all of the other logging code I assume). I'll change that over to a dash.
  2. Hmm...I click.unstyled the message before logging it to the file. I wonder if the logger formatter that was already included with tmuxp has some coloring that is left over. We probably don't want the color if it is leaving the (bash?) color codes. Or if we want to give the option for coloring the log file, I think it should be an additional option but defaults to no color. Thoughts?

I also just remembered that I forgot to update the README and other supporting documentation. I'll go ahead and do that as well

@tony tony merged commit 21b4639 into v1.6.x Nov 22, 2020
tony added a commit that referenced this pull request Nov 22, 2020
@tony
Copy link
Member

tony commented Nov 22, 2020

@joseph-flinn Sorry for the delay! I may be busy on the weekend for a week or two (errands on my end!)

Merged in v1.6.3 and v1.7.0a3!

1.6.3: http://localhost:8031/history.html#tmuxp-1-6-3-2020-11-22, https://pypi.org/project/tmuxp/1.6.3/

1.7.0a3: http://localhost:8031/history.html#tmuxp-1-7-0a3-2020-11-22, https://pypi.org/project/tmuxp/1.7.0a3/

@tony tony deleted the jf-load-logging branch November 22, 2020 20:37
@joseph-flinn
Copy link
Contributor Author

joseph-flinn commented Nov 22, 2020 via email

@tony
Copy link
Member

tony commented Nov 22, 2020

@joseph-flinn Sounds good!

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.

None yet

2 participants