coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

Implement seq (#45)

Open FrancisMurillo opened this issue 6 years ago • 10 comments

Implement seq.

Drafted the PR to show progress and any possible early reviews if needed.

TODO:

  • [ ] Unit tests
    • [ ] NaN ranges
  • [ ] Support other seq options
    • [ ] --format
    • [ ] --separator
    • [ ] --equal-width
  • [ ] Squash and rebase

FrancisMurillo avatar Oct 01 '19 13:10 FrancisMurillo

For next commits follow the git commit message guideline please

GrayJack avatar Oct 01 '19 15:10 GrayJack

Running cargo run --bin seq 10 -1.001 1 gives me this output:

10
8.999
7.998000000000001
6.997000000000002
5.996000000000002
4.995000000000003
3.994000000000003
2.993000000000003
1.992000000000003

Which is expected when creating floating point sequences because of how floating point numbers are added or subtracted. To address this precision loss, I'm planning to use rust_decimal instead of f64 in the iteration step. Is it valid to import a dependency or no dependencies at all?

FrancisMurillo avatar Oct 02 '19 08:10 FrancisMurillo

As part of the PR, I have to run the rustfmt. Running cargo fmt, it keeps giving me an error:

> cargo fmt -p seq
Could not parse TOML: expected an equals, found a newline at line 3

So checking out line 3 of rustfmt.toml, it has some trouble with the following lines:

...
# Could not parse TOML: expected an equals, found a newline at line 3
make_backup
...
# Could not parse TOML: duplicate key: `wrap_comments`
wrap_comments = true
# Could not parse TOML: invalid escape character in string: `y` at line 54
license_template_path = "Copyright {\y} Eric Shimizu Karbstein."
,,,
# invalid type: integer `2018`, expected string for key `edition`
edition = 2018

Commenting those lines, produces an error:

Warning: can't set `skip_children = false`, unstable features are only available in nightly channel.

I was able to make it run with cargo +nightly fmt. Is that valid or should I do something else to use rustfmt?

FrancisMurilloDigix avatar Oct 02 '19 13:10 FrancisMurilloDigix

I think it's ok to use nightly just for formatting

Also, I'm ok with adding crates when makes sense, and your use makes sense, but try to keep on a minimum

GrayJack avatar Oct 02 '19 16:10 GrayJack

@GrayJack I'm at a crossroad. Currently looking at implementing --format and --equal-width which is simply the printf functionality. My initial tactic was to use parse the printf format and convert it into print! format. However, Rust does not support dynamic print! formats, it must be a static string.

I came across runtime-fmt that may be a solution but requires nightly so out of the question. So another alternative is to use printf directly via libc. The tactic is to convert rust_decimal to f32 to c_double and use the format accordingly. I am conflicted whether this solution is the right direction. Another idea is to reimplement printf formatting for f32with pure Rust but might take longer.

Any suggested direction or idea for my dilemma?

FrancisMurilloDigix avatar Oct 08 '19 13:10 FrancisMurilloDigix

Still, using printf from c would require unsafe, so I don't think you should use them In the echo code, you will find a good part of formatting there, missing only \uHHHH, \UHHHHHHHH, %%, %b and %q

GrayJack avatar Oct 08 '19 13:10 GrayJack

So I should parse the printf format and implement the display myself? Should be fun.

Using this as a reference for the format

FrancisMurilloDigix avatar Oct 08 '19 13:10 FrancisMurilloDigix

Yeah, I think at one point it will be in the coreutil_core, since echo, seq and printf will use that. but for now it's ok to be a copy&paste plus additions

GrayJack avatar Oct 08 '19 14:10 GrayJack

Sorry I did not understand, what do you mean by "copy&paste plus additions"?

FrancisMurilloDigix avatar Oct 08 '19 22:10 FrancisMurilloDigix

Copy and paste the formatting code from echo, add the cases that are missing (additions)

GrayJack avatar Oct 08 '19 22:10 GrayJack