Rewrite more

[AMythicDev]

The more utility seems fairly incomplete. It does not use any modern terminal library like crossterm or termion Reading from standard input is also not implemented along with other flags that are present in the original GNU more. This PR intends to rewrite more using crossterm, adding all the missing flags and making it as close to the original more as possible

There are few differences in the command line interface from the original more like not having the + and - symbols in the -<number>, +<number> and +/<pattern> options from the original more. This is due to the non-standard way of arguments in GNU more, which clap does not allow building

[sylvestre]

please let me know when it is ready but it is great to see such progress here :)

[AMythicDev]

Sure. I would try to complete it ASAP

[sylvestre]

I am aware that it is a draft but here are some initial feedback ;)

[AMythicDev]

Thanks for telling me that. I would change it in further commits

[AMythicDev]

I have rewrote the basic usable version of more. Although none of the CLI flags are actually implemented and I will address them in a future PR.

I think that this PR should be merged before I start working on the CLI part. I see there's a merge conflict which I will resolve probably by tomorrow and squash everything so that we could merge this

[AMythicDev]

This PR is ready for merging. If you have any suggestions, you can comment it down here. I will address them in my next PR

[sylvestre]

Seems that a test is failing on Windows:

---- test_more::test_more_no_arg stdout ----
current_directory_resolved: run: D:\a\coreutils\coreutils\target\i686-pc-windows-msvc\debug\coreutils.exe more
thread 'test_more::test_more_no_arg' panicked at 'Command was expected to fail.
stdout = stderr = ', tests\common\util.rs:155:13
failures:
test_more::test_more_no_arg

[sylvestre]

it builds fine and bravo for the tests

However:

• more without any arguments cannot be exited?!
  Gnu is returning:

LANG=C more
more: bad usage
Try 'more --help' for more information.

• more is breaking the display of the terminal (reset is needed to come back in a normal state)

• more Makefile. the content isn't rendered correctly.
  For example on my system

USEGNU=gmake $*
all:
@$(USEGNU)
.DEFAULT:
@$(USEGNU)
% ╭─sylvestre@pinasil ~/dev/debian/coreutils ‹master*› ╰─$ 

• ./target/release/coreutils more non-existing
  is failing with:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', /home/sylvestre/dev/debian/coreutils/src/uu/more/src/more.rs:148:63
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[AMythicDev]

I think, your issues are probably fixed in the last commit, you could review it again and report if anything goes wrong

[sylvestre]

bad usage? This is the error that GNU is reporting?

[sylvestre]

It still breaks the terminal for me.

Example with zsh & gnome-terminal:

 [20:18:40]
╭─sylvestre@pinasil ~/dev/debian/coreutils ‹master*› ╰─$ ./target/release/coreutils more [20:18:42]
more: bad usage
Try 'more --help' for more information.
% ╭─sylvestre@pinasil ~/dev/debian/coreutils ‹master*› ╰─$ [./tacd dev/debian/coreutils cd: aucun fichier ou dossier de ce type: dev/debian/coreutils
% ╭─sylvestre@pinasil ~/dev/debian/coreutils ‹master*› ╰─$ [ls:18:48]
1602.patch Cargo.lock Makefile.toml
2020.patch Cargo.lock.orig README.md
2056.diff Cargo.toml sample.txt
2056.patch CODE_OF_CONDUCT.md sample.txt.orig
2057.patch CONTRIBUTING.md shuffled_wordlist.txt
2109.diff DEVELOPER_INSTRUCTIONS.md src
2109.diff.1 docs target
a examples tests
a~ GNUmakefile tmp
b LICENSE util
build.rs Makefile
% ╭─sylvestre@pinasil ~/dev/debian/coreutils ‹master*› ╰─$ [20:18:48]

[sylvestre]

Sorry but did you test your change?
Just running the tests is breaking my terminal
Try:

$ cargo test --release --features feat_os_unix
generate a long list of

 test test_unexpand::unexpand_init_1 ... ok
test test_unexpand::unexpand_init_list_0 ... ok
test test_unexpand::unexpand_init_list_1 ... ok
test test_unexpand::unexpand_read_from_two_file ... ok
test test_unexpand::unexpand_read_from_file ... ok
test test_unexpand::unexpand_spaces_follow_tabs_1 ... ok

[AMythicDev]

I fixed the issue with broken terminal. Could you recheck that.

[sylvestre]

Could you decrease the minimal version of crossterm, it fails to build with Rust 1.40 with:

error: attributes are not yet allowed on `if` expressions
--> /home/sylvestre/.cargo/registry/src/github.com-1ecc6299db9ec823/crossterm-0.19.0/src/command.rs:123:9
|
123 | #[cfg(windows)]
| ^^^^^^^^^^^^^^^
error: aborting due to previous error

[AMythicDev]

Sure. I will check that. This might take some time as I would probably have to go through a hit and trial process

[sylvestre]

To test
cargo +1.40.0 test --features unix

[sylvestre]

Besides that, it looks great :)

[AMythicDev]

I have downgraded the minimum version of crossterm and everything compiles correctly with Rust 1.40.

[tertsdiepraam]

This rewrite is awesome! Sadly, I'm getting some errors on my machine:

---- test_cp::test_cp_directory_not_recursive stdout ----
current_directory_resolved: run: /home/terts/Programming/coreutils/target/debug/coreutils cp hello_dir/ copy_of_hello_world.txt
thread 'test_cp::test_cp_directory_not_recursive' panicked at 'assertion failed: self.stderr_str().contains(cmp.as_ref())', tests/common/util.rs:303:9
---- test_more::test_more_dir_arg stdout ----
current_directory_resolved: run: /home/terts/Programming/coreutils/target/debug/coreutils more .
thread 'test_more::test_more_dir_arg' panicked at 'assertion failed: `(left == right)`
left: `"thread \'main\' panicked at \'called `Result::unwrap()` on an `Err` value: IoError(Os { code: 25, kind: Other, message: \"Inappropriate ioctl for device\" })\', src/uu/more/src/more.rs:179:33\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace"`,
right: `"more: \'.\' is a directory.\nTry \'more --help\' for more information."`', /home/terts/Programming/coreutils/tests/by-util/test_more.rs:21:9

These seem to be caused by a bug in crossterm that was fixed in version 0.18, exactly the version we can't use because Rust 1.40 does not allow attributes on if expressions. As far as I can tell, Rust 1.43 is the lowest Rust version that supports attributes on if (implemented by this PR).

Apart from this error, really nice job, it looks great when I use the recent versions of crossterm!

@sylvestre Do you think we could set the MSRV to 1.43 to use crossterm and attributes on if? Another driver is that 1.41 introduces the new Cargo.lock format (so no more cargo +1.40.0 update hopefully)

[tertsdiepraam]

I've fixed some things up and I think this can be merged now (if the CI is green).

There are a couple of remaining issues that should be dealt with in other PR's:

• unicode width needs to be used instead of line.len()
• there are many unimplemented arguments
• the name of the next file should be shown in the prompt when the end of a file is reached

I'll create issues for these once this is merged.

@arijit79 This PR is amazing, thanks!