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

Implement xpns_log() #192

Merged
merged 17 commits into from
Apr 25, 2023
Merged

Implement xpns_log() #192

merged 17 commits into from
Apr 25, 2023

Conversation

ReDTerraN
Copy link
Contributor

@ReDTerraN ReDTerraN commented Apr 18, 2023

Introducing xpns_log()
This will replace following functions

  • xpns_msg()
  • xpns_msg_info()
  • xpns_msg_warning()
  • xpns_msg_error()
  • xpns_msg_debug()

This will consolidate our code and make it easier as its all in one function.

Features
loglevels is now called via an positional argument, eg:
xpns_log "error" "${_dir} is not writable."

we now have following values we can call:

  • info
  • warning
  • error
  • debug

New look to the logoutput
It now comes with a new updated output to improve readability

old vs new
xpanes:Info: This is an info message
xpanes:Debug: [2023-04-18_15:24:33]:main:debug example

[xpanes:info] This is an info message
[xpanes:debug:2023-04-18_15:24:33:main] debug example

Ability to parse multiple inputs

old function had the limitations to only parse the first positional argument it acquired. With xpns_log we can make how many arguments we wish.

For example:

old and new
xpns_msg_info "this is an info message" "and here is more information"
xpns_log "info" "This is an info message" "and here is more information"

this provides following input:

old
xpanes:Info: this is an info message
new
[xpanes:info] This is an info message and here is more information

I have tested the function on all the input in xpanes, but should we ever get an issue with parsing the loglevel, we have a following output:
[xpanes:internal error] invalid log type, if you get this error. Please file an issue on github: https://github.com/greymd/tmux-xpanes/issues

Referring to setup an issue on the project if it ever occurs.
I have also made unit testing for the function to make sure we catch any issues.

Cheers

@ReDTerraN
Copy link
Contributor Author

hmm, my unit test and shfmt tests works locally, But it does not work in the CI/CD pipeline. I will try later this week troubleshoot why it is the case.

@ReDTerraN ReDTerraN marked this pull request as draft April 18, 2023 18:23
@ReDTerraN ReDTerraN marked this pull request as ready for review April 24, 2023 20:27
@ReDTerraN
Copy link
Contributor Author

All dependencies should now have been secured.
But im unable to satistfy shfmt and I do not get an hint to what I need to fix.

Running the command locally provides me no hint to what is needed

shfmt -i 2 -ci -sr -kp -d ./bin/xpanes ./*.sh
 echo $?
0

@greymd If you can assist that would be greatly helpful, other than that I now believe its ready to reviewed.

Thanks :)

@greymd
Copy link
Owner

greymd commented Apr 24, 2023

Running the command locally provides me no hint to what is needed

Oh really, ok I am taking a look..

bin/xpanes Outdated Show resolved Hide resolved
@ReDTerraN
Copy link
Contributor Author

ReDTerraN commented Apr 25, 2023

Thanks @greymd, I have now applied the docker container to my workflow instead of running it locally. All checks should now be done.

@greymd greymd merged commit 68fc1b6 into greymd:master Apr 25, 2023
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.

2 participants