Skip to content

Conversation

@katcipis
Copy link
Member

@katcipis katcipis commented Nov 10, 2020

Not sure if exposing a Log method is the best way to go, but wanted to respect the debug configuration.

Not sure if exposing a Log method is the best way
to go, but wanted to respect the debug configuration.
@katcipis katcipis marked this pull request as draft November 10, 2020 15:55
The idea is to make it more explicit that on error situations
the offset returned is zero. The function was doing that but
in a way that we had to check if the offset var was
not being assigned to some value, in the end it never is,
it is always 0 (or I got something wrong).
The var was never initialized, maybe it could be some sort
of empty/default thing, but the name was at least misleading.
Since the function returns a slice just using nil for empty
slice is idiomatic in Go and should work.
@katcipis katcipis changed the title WIP: Improve logging on Completer Improve logging on Completer Nov 10, 2020
@katcipis katcipis requested review from i4ki and vitorarins November 10, 2020 17:47
@katcipis katcipis self-assigned this Nov 10, 2020
@katcipis katcipis marked this pull request as ready for review November 10, 2020 17:49
@katcipis katcipis requested a review from lborguetti November 10, 2020 17:58
@codecov-io
Copy link

codecov-io commented Nov 10, 2020

Codecov Report

Merging #285 (4160571) into master (9ce2534) will decrease coverage by 0.56%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
- Coverage   42.32%   41.75%   -0.57%     
==========================================
  Files          52       52              
  Lines        7318     6378     -940     
==========================================
- Hits         3097     2663     -434     
+ Misses       3931     3429     -502     
+ Partials      290      286       -4     
Impacted Files Coverage Δ
cmd/nash/completer.go 0.00% <0.00%> (ø)
internal/sh/shell.go 62.80% <0.00%> (-2.52%) ⬇️
nash.go 40.35% <0.00%> (-4.10%) ⬇️
stdbin/mkdir/main.go 10.00% <0.00%> (-13.08%) ⬇️
internal/sh/log.go 50.00% <0.00%> (-10.00%) ⬇️
ast/tree.go 30.00% <0.00%> (-8.47%) ⬇️
internal/sh/builtin/len.go 53.84% <0.00%> (-7.27%) ⬇️
tests/cfg.go 46.66% <0.00%> (-6.28%) ⬇️
internal/sh/cmd.go 74.00% <0.00%> (-4.34%) ⬇️
readline/password.go 8.33% <0.00%> (-4.17%) ⬇️
... and 45 more

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 9ce2534...4160571. Read the comment docs.

Copy link
Member

@vitorarins vitorarins left a comment

Choose a reason for hiding this comment

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

💯

@i4ki
Copy link
Collaborator

i4ki commented Jan 12, 2023

wow this is old, sorry =(

@katcipis katcipis merged commit f228452 into master Jan 13, 2023
@katcipis katcipis deleted the improveAutocompleteLogging branch January 13, 2023 00:30
@rootshell2
Copy link

rootshell2 commented Jan 13, 2023 via email

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.

6 participants