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

Make chrono optional #438

Open
sigmaSd opened this issue Jun 5, 2022 · 3 comments
Open

Make chrono optional #438

sigmaSd opened this issue Jun 5, 2022 · 3 comments
Labels
A-API Area: Changes to the consumer facing API A-Performance Area: Speed and resource footprint of reedline to guarantee a pleasant smooth experience enhancement New feature or request

Comments

@sigmaSd
Copy link

sigmaSd commented Jun 5, 2022

Chrono is a relatively big dependency, at least making it optional would allow user to choose to remove it

cargo bloat: chrono alone is adding 1.1% to text section

File  .text    Size    Crate Name
0.1%   1.2% 19.4KiB      std addr2line::ResDwarf<R>::parse
0.1%   1.1% 18.4KiB      std std::backtrace_rs::symbolize::gimli::resolve::{{closure}}
0.1%   0.7% 11.7KiB reedline reedline::engine::Reedline::handle_editor_event
0.1%   0.6% 10.7KiB reedline reedline::main
0.0%   0.6%  9.5KiB      std addr2line::ResUnit<R>::parse_lines
0.0%   0.6%  9.4KiB   chrono <chrono::format::strftime::StrftimeItems as core::iter::traits::iterator::Iterator>::next
0.0%   0.5%  8.5KiB      std miniz_oxide::inflate::core::decompress
0.0%   0.5%  7.6KiB   chrono chrono::format::format_inner

here is an example implementation https://github.com/nushell/reedline/compare/main...sigmaSd:opt_chrono?expand=1

comparison with this pr

# with chrono
cargo tree | wc -l
> 88
# without chrono
 cargo tree --no-default-features | wc -l
> 77
@sigmaSd sigmaSd added the enhancement New feature or request label Jun 5, 2022
@sholderbach
Copy link
Member

Do you happen to know an alternative light weight crate for that task? In this case it is just needed for the basic timestamp in the example Prompt and now in the History with metadata (#401)

@phiresky
Copy link
Contributor

phiresky commented Jun 6, 2022

that can probably also be done with std::time::SystemTime::now().duration_since(SystemTime::UNIX_EPOCH)
(same for the use for database history in nushell itself)

@sholderbach
Copy link
Member

For the time-stamping in the db that should be fine (if timezone or strict ordering concerns are not an issue). I don't recall the std library directly providing the formatting machinery for it to be useful in the example prompt (but yeah making a dependency for that a disable-able default feature could be an option for the dependency conscious)

@sholderbach sholderbach added A-Performance Area: Speed and resource footprint of reedline to guarantee a pleasant smooth experience A-API Area: Changes to the consumer facing API labels Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-API Area: Changes to the consumer facing API A-Performance Area: Speed and resource footprint of reedline to guarantee a pleasant smooth experience enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants