Replace scrut(1) with fileis(1) #146

trinity · 2024-07-15T14:30:11-06:00

trinity commented

2024-07-15 14:30:11 -06:00

Fixes #104.

This branch renames scrut(1) to fileis(1) as discussed in #104. Additionally, it rewrites fileis(1) in Rust.

Originally the branch was going to create a libfileis(3) which would be used from a Rust fileis(1), but it turns out every feature can be accomplished with the Rust standard library (thank you Emma for pointing this out) so I was able to rewrite it without making a library too.

Fixes #104. This branch renames scrut(1) to fileis(1) as discussed in #104. Additionally, it rewrites fileis(1) in Rust. Originally the branch was going to create a libfileis(3) which would be used from a Rust fileis(1), but it turns out every feature can be accomplished with the Rust standard library (thank you Emma for pointing this out) so I was able to rewrite it without making a library too.

trinity added the

enhancement

label 2024-07-15 14:30:11 -06:00

trinity added 5 commits 2024-07-15 14:30:12 -06:00

libfileis(3): a library for scrutinizing files 71655a8559

scrut(1): get rid of -h 59fa3ed3d2

scrut(1): banish gotos 2c4349872c

scrut(1): use libfileis b9c4b49603

scrut(1): fix ugly opt parsing 0f12dcc552

emma closed this pull request

2024-07-15 14:34:24 -06:00

emma reopened this pull request

2024-07-15 14:34:27 -06:00

emma commented

2024-07-15 14:34:53 -06:00

I didn’t mean to close this.

Is there an issue describing what is happening in this PR?

I didn’t mean to close this. Is there an issue describing what is happening in this PR?

trinity added 3 commits 2024-07-18 20:45:26 -06:00

fileis(1): scrap libfileis(3), work on rewriting scrut(1) in Rust 0819eeb75d

fileis(1): feature parity with C scrut(1) 02b5edae05

fileis.1: rename from scrut.1 958f08bd9e

trinity changed title from ~~WIP: libfileis(3)~~ to fileis(1)

2024-07-18 20:46:11 -06:00

trinity requested review from emma 2024-07-18 20:49:08 -06:00

trinity added 1 commit 2024-07-18 20:50:19 -06:00

libfileis(3): finish removal 6d7173e438

emma added a new dependency 2024-07-20 10:06:27 -06:00

#96 testing

trinity changed title from ~~fileis(1)~~ to Replace scrut(1) with fileis(1)

2024-07-26 07:52:03 -06:00

emma commented

2024-09-04 23:05:10 -06:00

Should this program return EX_IOERR on line 59 and EX_SOFTWARE on line 81?

Should this program return `EX_IOERR` on line 59 and `EX_SOFTWARE` on line 81?

emma reviewed 2024-09-05 00:33:40 -06:00

emma left a comment

@trinity: In the middle of this review I decided to simply make a patch and send it your way via instant messaging.

src/fileis.rs Outdated

						
				@ -0,0 +37,4 @@

				fn main() -> ExitCode {

					let argv = args().collect::<Vec<String>>();

					let mut sel = String::from(""); // selected options

emma commented

2024-09-04 23:08:45 -06:00

let mut sel = String::new();

`let mut sel = String::new();`

emma marked this conversation as resolved

emma commented

2024-09-05 00:34:01 -06:00

Should this program return EX_IOERR on line 59 and EX_SOFTWARE on line 81?

I see now that line 81 should stay the same.

> Should this program return `EX_IOERR` on line 59 and `EX_SOFTWARE` on line 81? I see now that line 81 should stay the same.

trinity commented

2024-09-09 07:45:01 -06:00

Line 59 happens when a file is non-existent, which means it's failed the test.

emma commented

2024-09-10 02:23:22 -06:00

Line 59 happens when a file is non-existent, which means it's failed the test.

I see now that line 59 should stay the same.

> Line 59 happens when a file is non-existent, which means it's failed the test. I see now that line 59 should stay the same.

emma added 1 commit 2024-09-10 02:43:03 -06:00

fileis(1): various changes to make the code more efficient and idiomatic f4bd4de2e4

trinity reviewed 2024-09-25 22:18:15 -06:00

src/fileis.rs

						
				@ -0,0 +55,4 @@

					if optind == argv.len() { return usage(&argv[0]); }

					for arg in argv.iter().skip(optind) {

						let fmeta = match metadata(arg) {

trinity commented

2024-09-25 22:18:15 -06:00

I would rather have declarations precede assignments here. I understand assignment on declaration is idiomatic in Rust. Python, in fact, due to its type system, mandated it. But I've been forced to separate them (often) in C, and, having used both styles, prefer the separation for clarity of code.

emma commented

2024-09-27 02:58:11 -06:00

The separation decreases clarity, as it separates the context from the declaration of the variable. Separation should occur only when necessary, in my opinion.

silt commented

2024-09-27 03:14:45 -06:00

firmly siding with emma on this. declaring variables at the top of the scope is an archaic convention imposed by the technical limitations of a bygone era. variables should, for readability's sake, generally be declared either near or at the site of their assignment, not way up at the top of the function (and in an entirely different scope!!).
the note about scoping is actually more significant now that we're arguing about rust instead of c. the rust compiler is able to do a far better job at optimization if you stick to sane conventions and, as far as i'm aware (and please do correct me if i'm mistaken), this is one of those cases where things end up better-optimized if you write normal rust code instead of fortran.

firmly siding with emma on this. declaring variables at the top of the scope is an archaic convention imposed by the technical limitations of a bygone era. variables should, for readability's sake, generally be declared either near or at the site of their assignment, not way up at the top of the function (and in an entirely different scope!!). the note about scoping is actually more significant now that we're arguing about rust instead of c. the rust compiler is able to do a far better job at optimization if you stick to sane conventions and, as far as i'm aware (and please do correct me if i'm mistaken), this is one of those cases where things end up better-optimized if you write normal rust code instead of fortran.

trinity commented

2024-09-29 20:47:28 -06:00

I disagree with the form but can't help agreeing with the compilation benefits.

trinity marked this conversation as resolved

emma approved these changes 2024-10-22 18:17:15 -06:00

This pull request has changes conflicting with the target branch.