Skip to content

Add a doc section on cabal list-bin. #7964

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

Merged
merged 8 commits into from
Feb 16, 2022

Conversation

philderbeast
Copy link
Collaborator

This is a documentation only change for #7774.

@Mikolaj Mikolaj requested a review from ulysses4ever February 9, 2022 09:34

$ cabal list-bin cabal-install
cabal: The list-bin command is for finding a single binary at once. The target
'cabal-install' refers to the package cabal-install-3.7.0.0 which includes the
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe I should drop the version suffix from cabal-install-3.7.0.0?

Copy link
Member

Choose a reason for hiding this comment

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

You are showing real output, so I guess a suffix is fine. Perhaps you could fake it to 3.6.2 so that users don't wonder why they can't find this secrete paid version on Hackage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dropped the version suffix to avoid the docs looking stale and out of date when we're at cabal-3.8.0.0 or later.

Copy link
Member

Choose a reason for hiding this comment

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

Good call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think inventing something (e.g. erasing some places) is worse than having an outdated version, following the principle of least surprise. Having older version in the example is less surprising to anyone reading this, I think.

But I don't feel strongly about it, it's just s nuisance. So fell free to leave it as is.

Copy link
Collaborator Author

@philderbeast philderbeast Feb 10, 2022

Choose a reason for hiding this comment

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

@ulysses4ever I had tried cabal-install-_._.0.0 (as in don't care about version number) but that looked awkward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seeing as # is commonly used to refer to number, we could go with cabal-install-#.#.0.0 or cabal-install-#.#.#.#.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, # is probably a bit better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I put back the version suffix with hashes in place of the actual digits.

Comment on lines 5 to 6
legacy sections. We talk in detail about all of the new-style project commands,
``help`` and ``list-bin`` but there are other commands.
Copy link
Member

Choose a reason for hiding this comment

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

How about something like " We discuss the help command, which is a global command, then we describe some arguments and flags that are common to many cabal commands, then we overview a package command list-bin and finally all of the new-style project commands."?

And then, if that was your intention, "Note that there are also other commands, which help doesn't list in the three sections, but that are rarely used in recommended workflows.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd claim that this intro is not needed at all. There's a table of contents and it's much easier to use Ctrl+F than to process this kind of descriptions (and I don't believe that anyone will try to read it from from cover to cover). Again, not a strong feeling, just my 5¢.

That said, Mikolaj's suggestions does sound better to me.

Copy link
Collaborator Author

@philderbeast philderbeast Feb 10, 2022

Choose a reason for hiding this comment

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

@Mikolaj when I started on this, the docs claimed to "give an in-depth description of all the commands" but this wasn't so. I included the help command output as a cheap way to show an abbreviated table of contents for cabal-commands.rst for commands that we do discuss.

Doing a quick check, it looks like commands fetch, info, list, user-config, upload, check, get, hscolour and report are not discussed. The init command is discussed in the quickstart of developing-packages.rst so maybe we should link to it? Same goes for the outdated and gen-bounds commands that are discussed in cabal-package.rst.

I see that hscolour is deprecated. Should we move it alongside the v1 stuff (as a separate issue and PR)?

$ cabal help hscolour
Generate HsColour colourised code, in HTML format.

Usage: cabal hscolour [FLAGS]

Requires the hscolour program.

Deprecated in favour of 'cabal haddock --hyperlink-source'.

I know this is feature creep for this PR, but I've search through https://cabal.readthedocs.io/en/3.6 for "alias" and couldn't find a hit. Would this also be a good time to add a table explaining the forward and legacy aliases?

Copy link
Member

Choose a reason for hiding this comment

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

@philderbeast: thank you very much for checking. I wasn't aware we overpromise and underdeliver so excessively. :D

As a first approximation, perhaps just change "all" to something like "some" re described commands? I like the inclusion of the help output --- it both works as a table of contents and the demonstration of the first item in the table. Given your findings, indeed, please remove my optimistic assurances that the commands we skip are not recommended. Links to command descriptions in other sections would be great.

Yes, please open a ticket or PR for hscolour! And I think searching in https://cabal.readthedocs.io/en/latest/ may be a good idea (it's regenerated from our CI all the time). I'd open a new ticket (or is here one?) or PR for the aliases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Mikolaj I side-stepped trouble I had previewing these changes by copying the cabal-commands.rst over to the drafts folder of the hakyll setup I have for my blog where pandoc can render a preview for me. Might we be able to setup a gitpod link or codespaces link so that contributors can work on the docs without having to do a local setup (I'm having some python trouble setting up sphinx even though I've had this going locally in the past.).

I will open other issues or PRs as you suggest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As a first approximation, perhaps just change "all" to something like "some" re described commands?

I made that change @Mikolaj.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've spun off two separate issues from this thread, #7975 and #7978.

Copy link
Member

Choose a reason for hiding this comment

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

Splendid. Please let me know and I will re-review.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I've included each suggestion (but not resolved conversations - should I?). Thanks @Mikolaj this PR is ready for re-review.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thank you. I guess, we don't have an etiquette re conversations --- personally I wait for the reviewer to resolved as "satisified" instead of resolving myself as "addressed".

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the contribution! I think it's a great start and improves things even in the current state. Some minor suggestions are inline.

One general point I have: the ratio between the text and cabal outputs is skewed towards output. I think there should be more human-readable text outside of quotes from cabal output to
provide more context and some cues for what to look out for in the output. Some of my comments try to improve on that.


.. warning::

``list-bin`` won't accept any ``all`` scope such as ``all:exes`` or
Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think @fgaz recently said somewhere that cabal list-bin all:exes works if there is only one exe and I just tried it and it does work. Probably doesn't if there are more (or zero?) exes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the catch. I was trying out the behaviour of list-bin with cabal itself, a big cabal.project. I should review the tests for list-bin too while I'm on it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got cabal list-bin all and cabal list-bin all:exes to work on some other projects too so I removed the warning. @Mikolaj can you please review this for merge now?

@ulysses4ever
Copy link
Collaborator

Since #7925, it'd be great to amend this PR with the mention of support for scripts.

@philderbeast
Copy link
Collaborator Author

philderbeast commented Feb 16, 2022

Since #7925, it'd be great to amend this PR with the mention of support for scripts.

I'd rather have that as a separate issue to work on later if that is alright with you @ulysses4ever.

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

👌

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Feb 16, 2022

@philderbeast sounds good. We should make sure to not forget to create such an issue :-) oh, it’s already up, brilliant!

@Mikolaj Mikolaj removed the merge me Tell Mergify Bot to merge label Feb 16, 2022
@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Feb 16, 2022

rebase

❌ Pull request can't be updated with latest base branch changes

Mergify needs the author permission to update the base branch of the pull request.
cabalism needs to authorize modification on its head branch.
err-code: F5D0C

@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

Mergify needs the author permission to update the base branch of the pull request.
cabalism needs to authorize modification on its head branch.

Huh. @philderbeast, what is 'cabalism'?

@Mikolaj Mikolaj added the squash+merge me Tell Mergify Bot to squash-merge label Feb 16, 2022
@philderbeast
Copy link
Collaborator Author

Huh. @philderbeast, what is 'cabalism'?

Just a github org where I keep my cabal related stuff, hpack-dhall and other stuff.

@philderbeast
Copy link
Collaborator Author

Sorry @Mikolaj I thought I was on the master branch on my fork and hit fetch but I was on branch doc/list-bin-7774 and it did a merge (this is through github's web UI). I would have rebased if I wasn't button trigger happy. Is that a problem?

@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

That's a tragedy, a disgrace and git-art, but it can be rectified either through squash (hence the "squash+merge me" label) or another rebase which should automatically remove the merge commit.

@jneira jneira linked an issue Feb 16, 2022 that may be closed by this pull request
@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

BTW, "squash+merge me" won't work, again, if 'cabalism' org does not give others the permission to modify head branch.

@Mikolaj Mikolaj added merge me Tell Mergify Bot to merge and removed squash+merge me Tell Mergify Bot to squash-merge labels Feb 16, 2022
@mergify mergify bot merged commit 19468bf into haskell:master Feb 16, 2022
@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

Success. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

list-bin is missing from user's manual
3 participants