search

GrayJack / coreutils

Core utils re-implementation for UNIX/UNIX-like systems written in Rust
Mozilla Public License 2.0
107 stars 40 forks source link

Seq: Major rewrite #154

Closed gahag closed 2 years ago

gahag commented 3 years ago

The issue #133 was quite hard to fix on the code as is, so I kind of made a major rewrite. The main strategy to prevent the precision issue, which is used in GNU Seq, is to compare the displayed numbers in order to identify if the last one should be printed. The code now adopts the same strategy, by leveraging the Display trait and keeping track of the last displayed number. The separator/terminator is output separately.

Please let me know if you have any suggestions or concerns.

gahag commented 3 years ago

@GrayJack can you add the hacktoberfest-accepted label to this please? Salve :)

GrayJack commented 2 years ago

@gahag Sorry for the late reply, I had to check a few things. I have a question before I go forward to review the code.

Did you look or used GNU seq code as reference to implement this rewrite?

gahag commented 2 years ago

For the rewrite, absolutely not. I only took a brief look at GNU seq prior to asking to work on the issue, for I wondered how it circumvented the issue, which is inherent to floating point arithmetic.

GrayJack commented 2 years ago

@gahag I ended my review, I have nothing to point out code related. But I noticed that you didn't put yourself as author. It's more as a recognition of work than anything, so we don't mind if you use real names or nicknames, you can use whatever you feel comfortable. It's also ok you don't want to add at all

Also, if you don't mind, can you rebase so the CI runs correctly? It was a crate that changed the required rust version on patch release

gahag commented 2 years ago

@GrayJack thanks! I have added myself as co-author and rebased into origin/dev.

GrayJack commented 2 years ago

bors r+

bors[bot] commented 2 years ago

Build succeeded: