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

Extra tweaks to CLI #72

Merged
merged 8 commits into from
Jan 9, 2024
Merged

Extra tweaks to CLI #72

merged 8 commits into from
Jan 9, 2024

Conversation

Vlix
Copy link
Collaborator

@Vlix Vlix commented Oct 7, 2023

Added a bunch of documentation, help strings and restructured the internals a bit.

@cdepillabout Take your time reviewing this. There's no rush.

Also, I'm wondering if it's even necessary to publish password-cli to hackage.com 🤔
It's only an executable, and hackage is for libraries. It being in the repo is good enough for anyone who'd like to build it, right?

Should we maybe add the fact that there IS a CLI to the password/README and link to the repo in there?
(When we're happy with the state of the CLI, of course)

Refactored the structure of the options to be a bit more modular:

* Moved '--quiet' flag to top of options
* Added extra flag to specificy literal reading of files, or only the first line.
* Drop whitespace around hash/password if only first line of a file is read
* Added lots of 'help' text to give more info when using '--help'
* Added '--version' option
* Add newline after outputting hash when using stdin interactively
@Vlix Vlix requested a review from cdepillabout October 7, 2023 14:27
@cdepillabout
Copy link
Owner

Thanks, I'll try to take a look at this in the next couple of days.

Maybe @blackheaven would want to review this as well?

@blackheaven
Copy link
Contributor

Also, I'm wondering if it's even necessary to publish password-cli to hackage.com 🤔
It's only an executable, and hackage is for libraries. It being in the repo is good enough for anyone who'd like to build it, right?

IIUC, it's the easiest way to make it available to everyone (cabal install and nixpkgs).

@Vlix
Copy link
Collaborator Author

Vlix commented Oct 8, 2023

@blackheaven Do you know of other executables on hackage that only have an executable stanza in their cabal file?

@blackheaven
Copy link
Contributor

@cdepillabout
Copy link
Owner

It's only an executable, and hackage is for libraries. It being in the repo is good enough for anyone who'd like to build it, right?

I think this is generally a (widely-held) misconception. Hackage is for distributing both libraries and standalone executables.

Like @blackheaven said, there are a few Linux distros that have automation around packaging/distributing Haskell packages from Hackage. But Haskell packages that aren't on Hackage don't benefit from this automation. As far as I know, Nixpkgs and OpenSUSE both fall into this category.

Comment on lines 38 to 42
```
user@computer $ cat password.txt | password-cli hash pbkdf2
Enter password:
sha512:25000:8ZJ1T55Y0sPRwltXNe/2fA==:aA0BT1WlTg+t2pSr8E6+l2zJW88rmUiDlKeohSOnzS0nLOumDSyK0FfsiNJBvWvWVkB2r6IMxRqelk4LZR33ow==user@computer $
```
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
```
user@computer $ cat password.txt | password-cli hash pbkdf2
Enter password:
sha512:25000:8ZJ1T55Y0sPRwltXNe/2fA==:aA0BT1WlTg+t2pSr8E6+l2zJW88rmUiDlKeohSOnzS0nLOumDSyK0FfsiNJBvWvWVkB2r6IMxRqelk4LZR33ow==user@computer $
```
```console
$ cat password.txt | password-cli hash pbkdf2
Enter password:
sha512:25000:8ZJ1T55Y0sPRwltXNe/2fA==:aA0BT1WlTg+t2pSr8E6+l2zJW88rmUiDlKeohSOnzS0nLOumDSyK0FfsiNJBvWvWVkB2r6IMxRqelk4LZR33ow==
```

I'd just remove the user@computer $ at the end of the line here, since you explain it in the next paragraph.

Although I see why you originally wrote it like this. It may be easier to understand as you have it in this current PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, but without the user@computer it's difficult to convey it obviously.
I removed it and hope it's obvious from the text and any testing people would do with the CLI.

@Vlix
Copy link
Collaborator Author

Vlix commented Oct 10, 2023

I was wondering what the highlight code for console was. bash didn't quite do the same 😅 so I kept it plain text.

@Vlix Vlix merged commit e535cda into cdepillabout:master Jan 9, 2024
18 checks passed
@Vlix Vlix deleted the extra-tweaks-to-cli branch January 9, 2024 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants