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

feat: improve display format of literals #1494

Merged
merged 5 commits into from
Mar 31, 2022
Merged

Conversation

fuyufjh
Copy link
Member

@fuyufjh fuyufjh commented Mar 31, 2022

What's changed and what's your intention?

Improve display format of literals.

Use fmt::Display instead of fmt::Debug, except that string literals are wrapped with single quotation marks (e.g. 'foo').

Why implement Display instead of ToString?

https://doc.rust-lang.org/std/string/trait.ToString.html

This trait (ToString) is automatically implemented for any type which implements the Display trait. As such, ToString shouldn’t be implemented directly: Display should be implemented instead, and you get the ToString implementation for free.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

None

@codecov
Copy link

codecov bot commented Mar 31, 2022

Codecov Report

Merging #1494 (b110d41) into main (def472a) will increase coverage by 0.04%.
The diff coverage is 73.91%.

@@             Coverage Diff              @@
##               main    #1494      +/-   ##
============================================
+ Coverage     70.49%   70.53%   +0.04%     
  Complexity     2766     2766              
============================================
  Files          1028     1029       +1     
  Lines         90195    90329     +134     
  Branches       1790     1790              
============================================
+ Hits          63585    63717     +132     
- Misses        25719    25721       +2     
  Partials        891      891              
Flag Coverage Δ
java 61.01% <ø> (ø)
rust 72.56% <73.91%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rust/common/src/types/chrono_wrapper.rs 86.84% <0.00%> (ø)
rust/common/src/types/decimal.rs 81.98% <40.00%> (ø)
rust/frontend/src/expr/literal.rs 65.45% <83.33%> (+3.45%) ⬆️
rust/common/src/types/interval.rs 56.03% <100.00%> (+0.86%) ⬆️
rust/common/src/types/mod.rs 61.64% <100.00%> (+2.40%) ⬆️
rust/stream/src/executor/chain.rs 0.00% <0.00%> (-47.25%) ⬇️
rust/storage/src/storage_value.rs 48.48% <0.00%> (-22.95%) ⬇️
rust/frontend/src/binder/bind_context.rs 88.13% <0.00%> (-8.48%) ⬇️
rust/common/src/util/ordered/serde.rs 93.01% <0.00%> (-6.52%) ⬇️
rust/stream/src/executor_v2/mod.rs 41.66% <0.00%> (-5.96%) ⬇️
... and 46 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Member

@xxchan xxchan left a comment

Choose a reason for hiding this comment

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

What makes Normalized(1.0) become 1.0? Interesting 🤔

@fuyufjh
Copy link
Member Author

fuyufjh commented Mar 31, 2022

What makes Normalized(1.0) become 1.0? Interesting 🤔

Debug -> Display, I think

@fuyufjh fuyufjh enabled auto-merge (squash) March 31, 2022 14:14
@fuyufjh fuyufjh merged commit c964573 into main Mar 31, 2022
@fuyufjh fuyufjh deleted the eric/fix_literal_display branch March 31, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants