Skip to content

Conversation

@johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Jun 13, 2022

This commit picks the result from JuliaImages/ImageInTerminal.jl#62

(Not yet done) In the spirit of LazyModules, we might also want to make the final API depends on basic Base types:

xterm_24bit_encode(img; kwargs...)
xterm_24bit_encode(io::IO, img; small_block=true)

Curiously, @t-bltg I noticed that you have explicitly changed a few IOBuffer usages to PipeBuffer in many places, what's the benefit of PipeBuffer over IOBuffer?

ImageBase was used in ImageInTerminal to recursively downsample the
image into a reasonable size. But for a pure encoding/decoding package,
we don't need such heavy dependency. ColorTypes is sufficient since we
only use the types for dispatch purposes.
@codecov
Copy link

codecov bot commented Jun 13, 2022

Codecov Report

Merging #3 (5d9210d) into master (4f06b40) will decrease coverage by 2.93%.
The diff coverage is 91.11%.

❗ Current head 5d9210d differs from pull request most recent head f4494fa. Consider uploading reports for the commit f4494fa to get more accurate results

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
- Coverage   98.09%   95.16%   -2.94%     
==========================================
  Files           3        4       +1     
  Lines         105      124      +19     
==========================================
+ Hits          103      118      +15     
- Misses          2        6       +4     
Impacted Files Coverage Δ
src/utils.jl 71.42% <71.42%> (ø)
src/colors.jl 93.54% <93.54%> (ø)
src/XTermColors.jl 100.00% <100.00%> (ø)
src/encoder.jl 97.36% <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 4f06b40...f4494fa. Read the comment docs.

@t-bltg
Copy link
Member

t-bltg commented Jun 13, 2022

Curiously, @t-bltg I noticed that you have explicitly changed a few IOBuffer usages to PipeBuffer in many places, what's the benefit of PipeBuffer over IOBuffer?

PipeBuffer is a simplified IOBuffer which does not support moving the cursor position (seek, ...). I think it's more performant, but I haven't tested it for perf. It simplifies the code such as https://github.com/JuliaLang/julia/blob/c298e4ee996c4d52a0ceb17c209831042f72e6c0/test/iobuffer.jl#L152: read(io, String) for a Pipebuffer vs String(take!(io)) for a IOBuffer.

@johnnychen94
Copy link
Member Author

I'll continue or follow up your devs the next day, so let me merge this first.

This commit introduces a few changes:

- the 256 encoding algorithm is revisited to give a visually more consistant
  result for decoding. This changes the numerical results and is a
  breaking change.
- 256 decoding is added
- the TermColorDepth type becomes a functor -- both an encoder and a
  decoder.
also git ignore .vscode folder
@johnnychen94 johnnychen94 merged commit 5b5aeba into master Jun 13, 2022
@johnnychen94 johnnychen94 deleted the jc/cleanup branch June 13, 2022 21:00
@johnnychen94 johnnychen94 mentioned this pull request Jun 14, 2022
Closed
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