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

Add subcommand help #10

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@maxnordlund
Copy link
Contributor

maxnordlund commented Apr 3, 2016

Just fixing a TODO, if I understood it correctly.

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully ran brew tests with your changes locally?
@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 3, 2016

Thanks for looking into this! I think this TODO was actually meant that at some point we want to make brew help <command> and brew <command> --help work properly and consistently—and more importantly—show help specific to the given command instead of just printing an overview. What that means is that the TODO you found is, in my understanding, related to this piece of code from brew.rb. Would you be interested in working on this?

I'm not sure we want to take your contribution as-is, because it would make the output of brew update --help more useful, but unfortunately inconsistent with brew help update.

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 3, 2016

Ah, I see. I'll take a look at right away. It seems like it shouldn't be to hard. However, where should the help text live? In a bunch of small files, or in the man page? The latter will make this a bit tricky, but having a bunch of small files introduces a build step for the man page.

@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 3, 2016

Ah, I see. I'll take a look at right away.

Thanks! If you can come up with a good solution for this long-standing issue, that would be much appreciated! ❤️

However, where should the help text live? In a bunch of small files, or in the man page?

That's not easily answered, but ideally we'll find a solution that avoids duplicating the documentation (as separate pieces of documentation tend to diverge easily over time). Maybe you can ponder the different possible options and their pros/cons and present them here. I don't want to stop you, but it might also be a good idea to wait a little bit for other maintainer's opinions.

[…], but having a bunch of small files introduces a build step for the man page.

I don't think that's a major issue. We already use brew man to regenerate the man page and a corresponding HTML file from the Markdown version of the man page. A step that combines multiple smaller files into a single file could be added to that.

@apjanke

This comment has been minimized.

Copy link
Contributor

apjanke commented Apr 3, 2016

I agree with Martin; I think we'd want to pass on this for now because of the brew help update vs brew --help update difference; as I understand, they're supposed to be identical. Thank you for the submission though!

That's not easily answered, but ideally we'll find a solution that avoids duplicating the documentation (as separate pieces of documentation tend to diverge easily over time).

This is a big question and I think it's tied to the need for better processing of command line options, where we'd like to be able to know which options are used by which commands, and provide completion help and feedback on unused/mistyped options.

I think a good way to do this would be to add support for defining brew subcommands as Ruby objects, instead of simple Ruby functions or scripts (with back-compatibility wrappers for the current style of commands and external commands). They could have helptext(), options(), summary() methods, and so on. I think that's probably necessary as a central way of handling the different ways commands can already be implemented now, and need to continue to be supported due to the "external command" mechanism. I'm actually working on a proof-of-concept of one way to do this now.

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 3, 2016

So, I've implemented this (more) properly, and currently regexping the brew.1.md file to do this. But for the long term this needs to live inside the commands, as you said.

Now both brew foo --help and brew help foo output the same thing, which is the corresponding sections from the man page.

@maxnordlund maxnordlund changed the title Add help to brew update Add subcommand help Apr 3, 2016

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Apr 4, 2016

I like that you're trying to make a start on subcommand help, well done. Rather that just trying to parse the manpage I think it may be worth having dedicated help perhaps in the file itself. The nicest pattern I've seen for that was using #/ as comments at the top of the file and then reading those lines just with grep and tr.

s/^ //; # Adjust the spacing a bit
p;
}' "$HOMEBREW_LIBRARY/Homebrew/manpages/brew.1.md"
exit 0 ;;

This comment has been minimized.

@UniqMartin

UniqMartin Apr 4, 2016

Contributor

This is not performance-critical and we don't have to implement this part in Bash, so I think it's best to keep this simple. Meaning: let's just make this redirect to brew help update instead of brew --help and let brew help <command> figure out where to get the help text from (so we have a single place that deals with fetching the help text).

This comment has been minimized.

@maxnordlund

maxnordlund Apr 4, 2016

Contributor

Yeah, I guess, but this is a second faster, not even kidding. So it had some benefit. But if we move the help test into the files them selfs, this definitely needs to go.

@xu-cheng

This comment has been minimized.

Copy link
Contributor

xu-cheng commented Apr 4, 2016

As suggested by @bfontaine, we probably should reimplement our command system using Command class. i.e. make each internal command like:

class InstallCommand < Command
  help <<-EOS.undent
    ...
  EOS

  def run
  end  
  ...
end
@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 4, 2016

Rather that just trying to parse the manpage I think it may be worth having dedicated help perhaps in the file itself. The nicest pattern I've seen for that was using #/ as comments at the top of the file and then reading those lines just with grep and tr.

I have to agree with Mike. I think this sounds like a nice pattern. It also keeps the help text close to the implementation, which should help with keeping the help text in sync with the code. It's initially a bit more work, as the individual parts of the man page need to be moved to the command files, but it sounds like this will pay off long term.

@maxnordlund If you agree this is a sensible approach, can you maybe do that for a few commands so that we get a better idea of how the code could look like? That would also require some adjustments to brew man to make it assemble the complete Markdown man page from those individual snippets, then proceed to convert this generated file to brew.1 and brew.1.html. If everyone is happy with how that looks, we could proceed to move the help texts for all commands.

As suggested by @bfontaine, we probably should reimplement our command system using Command class. i.e. make each internal command like:

I think this is a good long-term goal, but what is being suggested here is much easier to implement and provides an immediate benefit. It can be modified further once we're ready to provide a better abstraction for our commands, and then moving those help texts into an help <<-EOS.undent block is no big deal.

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 4, 2016

So I've changed it to pull the help text from the top level comment, like @MikeMcQuaid suggested. It does look a lot nicer, that sed was a bit much 😄

I've also pulled out the help logic into brew.sh to make it snappy, and since I need to read the files anyway, it seemed simpler then doing it in ruby.

I did go with #: to make rubocop happy, and I think this will replace help.rb completely.

then
HOMEBREW_COMMAND="$HOMEBREW_COMMAND.rb"
else
echo "Unknown command \`$@\`"

This comment has been minimized.

@bfontaine

bfontaine Apr 4, 2016

Member

How does this work with custom commands?

This comment has been minimized.

@maxnordlund

maxnordlund Apr 5, 2016

Contributor

Yeah, you're totally right. This should just pass through to allow external commands to handle help themselves.

This comment has been minimized.

@UniqMartin

UniqMartin Apr 5, 2016

Contributor

That's a good question. Currently things like brew <command> --help or brew --help <command> are handled somewhat transparently by invoking the external command with --help as an argument, thus giving it a chance to display its own help text.

echo "$HOMEBREW_HELP"
exit 1
fi
grep "#:" "$HOMEBREW_COMMAND" | cut -c3- | tr -d \` | sed "s/^\* //"

This comment has been minimized.

@bfontaine

bfontaine Apr 4, 2016

Member

You might want to grep on ^#: to ensure we don’t match e.g. strings that contain this pattern.

This comment has been minimized.

@bfontaine

bfontaine Apr 4, 2016

Member

Also it might be better to include “clean” text right away rather than use tr/sed at display time.

This comment has been minimized.

@maxnordlund

maxnordlund Apr 5, 2016

Contributor

Yes, the ^#: sounds better, but I think it's going to be very hard to add back the backticks in the markdown. So I'd rather strip them out here, and keep the documentation comment markdown to make the man page easier.

This comment has been minimized.

@UniqMartin

UniqMartin Apr 5, 2016

Contributor

Also it might be better to include “clean” text right away rather than use tr/sed at display time.

I think the intention here is for this to be Markdown, so that we can still assemble a complete man page by stitching together the individual help texts. But for display via brew help we want to remove some of the markup to make it more readable.

@@ -1,3 +1,51 @@
#:* `install` [`--debug`] [`--env=`<std>|<super>] [`--ignore-dependencies`] [`--only-dependencies`] [`--cc=`<compiler>] [`--build-from-source`|`--force-bottle`] [`--devel`|`--HEAD`] <formula>:

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 5, 2016

Member

I'd personally prefer #/ but don't feel strongly.

This comment has been minimized.

@UniqMartin

UniqMartin Apr 5, 2016

Contributor

@maxnordlund mentioned he was using #: to make RuboCop happy, so I suspect #/ raises some kind of warning? To me, both look equally readable and are thus fine.

This comment has been minimized.

@maxnordlund

maxnordlund Apr 5, 2016

Contributor

Yes, the rubocop for space after # doesn't complain for some special characters, see leading_comment_space.rb.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 5, 2016

Member

I think we can just silence this RuboCop warning and use #/.

@@ -1,3 +1,36 @@
read -d "" HOMEBREW_HELP <<EOS

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 5, 2016

Member

I think it'd be good to pull this into Library/bash/help.sh and source it here.

This comment has been minimized.

@maxnordlund

maxnordlund Apr 5, 2016

Contributor

Yes, sounds good.

echo "$HOMEBREW_HELP"
exit 0
fi
fi

This comment has been minimized.

@xu-cheng

xu-cheng Apr 5, 2016

Contributor

I'm 👎 to handle help flag on bash script. It's a duplication of the ruby version and fails to provide any advantage. In addition, it makes brew.sh more complicate than it used to be.

At the same time, I'm 👎 to use grep/cut to read help message. feel a little code smell to me.

Why cannot we handle all of these in ruby library and store help message using proper string rather than comments.

This comment has been minimized.

@xu-cheng

xu-cheng Apr 5, 2016

Contributor

To be more specifically, we could create help_<commandname> methods for each commands just like the way we implement each commands themselves.

Then we could simply use Homebrew.send "help_#{cmd.to_s.tr("-", "_").downcase}" to get the help message.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 5, 2016

Member

@xu-cheng We can pull it into another bash script. We're going to have to deal with providing help for shell command eventually.

Why cannot we handle all of these in ruby library and store help message using proper string rather than comments.

Maybe we can but PRs for that sort of handling still feels like a long way off and we'll still need to handle Bash command help.

This comment has been minimized.

@xu-cheng

xu-cheng Apr 5, 2016

Contributor

We're going to have to deal with providing help for shell command eventually.

For shell command, we could just redirect them by brew help <command>, i.e put help_update method in cmd/help.rb.

Sorry but using comment to store string and then grep them out feels code smell to me.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 5, 2016

Member

I've seen it used on multiple projects in the past, for what that's worth, and it's a good way of using code comments and user comments in the same file to keep things easily in sync.

This comment has been minimized.

@xu-cheng

xu-cheng Apr 5, 2016

Contributor

Yes, I could understand to put doc along side the implementation. However, by using comment we will face these problems:

  • Redundant. It introduces unnecessary IO and string manipulation.
  • Fragile. The extract logic won't be simple, hence it introduces possibility for outputting incorrect or bad format result.
  • Limit us to add actual comment in the head of file.
  • Limit us to dynamic process help string. If we were using ruby string, we could dynamic insert say version number or url based on OS.
  • It's not easy to access for other purpose, e.g. generating manpage.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 5, 2016

Member

I can't agree that it's fragile at all. This logic is extremely simple and it's reading a single file so the overhead is almost non-existent.

It's not easy to access for other purpose, e.g. generating manpage.

While it would be nice to do this: we're a long way off that point.

This comment has been minimized.

@UniqMartin

UniqMartin Apr 5, 2016

Contributor

In addition to what Mike said, with what I agree completely:

Limit us to add actual comment in the head of file.

I don't think this is limiting us at all. Normal comments will continue to be # <comment text> (notice the space) instead of starting with #:, so there's no ambiguity there.

Limit us to dynamic process help string. If we were using ruby string, we could dynamic insert say version number or url based on OS.

While this has the danger of creating yet another markup language, there's nothing stopping us from doing some simple replacements on patterns that are very unlikely to appear in the help text, e.g., @OSX_RELEASE@ gets replaced with 10.11.4 or whatever the current value is.

It's not easy to access for other purpose, e.g. generating manpage.

It's actually very easy. We already do some processing in brew man for conversion to a proper man page and its HTML version. There's nothing stopping us from stitching together the Markdown from all cmd/*.{rb,sh} files and adding a header and footer in a preprocessing step.

This comment has been minimized.

@xu-cheng

xu-cheng Apr 5, 2016

Contributor

OK, your guys have persuaded me that this is a good short term solution. But it has at least one shortage in long term, it stops us to support localization(I know no one is working on this because i18n need Ruby 2.0, just want to mention long term impact).

This comment has been minimized.

@bfontaine

bfontaine Apr 5, 2016

Member

Yeah it’s a good short-term solution; long term we’ll probably use a class-based solution.

In long term, I think we will move to use Command class to handle command, which will provide not only help message, auto manpage(i.e. turn brew.1.md to brew.1.md.erb) but more importantly solving the problem of handling ARGV.

Also automatic Bash/Zsh/etc shell completion scripts generation.

@apjanke

This comment has been minimized.

Copy link
Contributor

apjanke commented Apr 5, 2016

This isn't a bad short-term solution, but I think a more robust class-based solution wouldn't be that hard. I've put together a proof-of-concept at #28 for comparison. It lets you define the subcommand help using DSL that looks like that of Formula, which might be a nice consistency for Homebrew.

class ListBrewCmd < BrewCmdClass
  summary "List installed formulae, or files in installed formula"
  helptext <<EOS

brew list, ls - #{summary}

  brew list [--full-name]
  brew list --unbrewed
  brew list [--versions [--multiple]] [--verbose] [--pinned] <formulae>

Options:
  --full-name  Print fully-qualified formula names
  --unbrewed   List all files in brew prefix not installed by Homebrew
  --versions   Include formula version numbers
  --multiple   Only show formulae with multiple versions installed
  --pinned     Show versions of pinned formulae
EOS

  def run
    Homebrew.list
  end
end

module Homebrew
  def list
  # ... rest of command definition ...

Have a look and let me know what you think. It could also be expanded to incorporate the syntax discussed here if that's a significant win.

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 5, 2016

The biggest problem I foresee using a Ruby based approach is that it firmly treats shell based commands as secondary citizen. For external commands, this may not be a big problem iff they can handle help themselves. But as @apjanke said, if they ignore it and have side effects, it can turn south fast.

I would like to keep the approach as consistent as possible, and having a special top level comment solves that quite neatly.

@maxnordlund maxnordlund force-pushed the maxnordlund:brew-update-help branch from 8233607 to 1d03ceb Apr 5, 2016

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 5, 2016

So I've refactored it into help.sh, which I think looks a lot nicer. The only thing left in brew.sh is a small normalization to make both brew help foo and brew foo --help go to help.sh.

After thinking a bit more on the command class approach, it makes sense for making autocompletion work smoother, and some other goodies. It'll still make shell commands harder to write, but maybe that's a sacrifice we're willing to make?

exit 0
else
# pass through to let external commands handle help themselves
true

This comment has been minimized.

@bfontaine

bfontaine Apr 5, 2016

Member

I think you can skip this else altogether; no need to use true here.

@xu-cheng

This comment has been minimized.

Copy link
Contributor

xu-cheng commented Apr 6, 2016

that it firmly treats shell based commands as secondary citizen.

Sorry, but shell based commands are the secondary citizen for Homebrew. We won't turn every piece of commands into bash, which will only introduce maintenance and performance overhead. We only use it for special cases where we cannot use Ruby, like update and install-vendor-ruby(which will happen in the future). So even, I agree using comment is a good short term solution. I'm still 👎 to implement the logic in bash.

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Apr 6, 2016

I'm still to implement the logic in bash.

I agree broadly but there's a few command where I can see an argument for doing it in future for speed reasons e.g. --prefix with a formula argument or cleanup (which I suspect we can make a lot, lot faster than it is currently).

@maxnordlund maxnordlund force-pushed the maxnordlund:brew-update-help branch from a5b1a22 to 8ab5dcf Apr 7, 2016

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 7, 2016

I've moved back the logic into ruby, which I guess makes sense. But about shell commands being secondary citizen, you also have to think about external commands. If we want the handling of help to be shared with those as well, it makes a lot of sense (at least to me) to make it as language agnostic as possible. By having this comment syntax, ruby, shell and python are naturally covered.

For other language they can do something like:

/**
#:* `foo` [OPTIONS] [FILES]
#:  ...
*/
@xu-cheng

This comment has been minimized.

Copy link
Contributor

xu-cheng commented Apr 7, 2016

you also have to think about external commands.

For external commands, the help flag should be handled by themselves as whatever they want.

More importantly, we should not introduce similar comment based help message to external command. Otherwise, this will become part of public API and I'm still considering this PR as short-term solution(i.e. it will change in the future).

# Handle both internal ruby and shell commands
require "cmd/help"
help_text = Homebrew.help_cmd(cmd)
puts help_text.empty?

This comment has been minimized.

@xu-cheng

xu-cheng Apr 7, 2016

Contributor

This should not be there?

This comment has been minimized.

@maxnordlund

maxnordlund Apr 7, 2016

Contributor

Whoops, yeah I'll fix that right away.

@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 8, 2016

I've updated the brew man command to extract the sub commands' help from their files, and added the remaining top level comments, so this should be good to go pending review I think.

I added a whole lot of comments and would like to see them addressed, but most of them are minor and of stylistic nature. What that means: I like very much what I'm seeing! Good job thus far! I think we're getting close to the point where this can get merged.

One more thing I'd like to see you do before we finalize this: If you're comfortable working with git and rewriting history, could you please consolidate the commits as follows?

  • 1st commit: Actual code changes, i.e. primarily Library/brew.rb, cmd/help.rb, and cmd/man.rb.
  • 2nd commit: Moving help texts into individual files in cmd/*.{rb,sh} and minor adjustments to those.
  • 3rd commit: Add regenerated brew.1.md, brew.1, and brew.1.html after running brew man.

Does organizing things like this make sense to you?

@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 8, 2016

How about making it brew.1.md.erb? So we will have one single file with the power of erb.

I'm not opposing this suggestion, but I'm wondering whether we really need this added complexity. The current approach of a header and footer is certainly much easier to understand. Can you make a case that is more compelling than “one file instead of two”?

@xu-cheng

This comment has been minimized.

Copy link
Contributor

xu-cheng commented Apr 8, 2016

Can you make a case that is more compelling than “one file instead of two”?

My thinking is it would make brew.1.md.erb more easy to read as it will show clear structure on how manpage is organized without reading the code of brew man. But sure, this can be done in a follow-up PR.

end

cmd_path.read.
split("\n").

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 8, 2016

Member

.lines does this

split("\n").
grep(/^#:/).
map { |line| line.slice(2..-1) }.
join("\n")

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 8, 2016

Member

Could be worth unifying this logic with the one in brew.rb somehow.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 8, 2016

Member

I mean help.rb but it probably wants to have some utility class instead.

@@ -313,22 +313,26 @@ If \fB\-\-versions\fR is passed, show the version number for installed formulae,
If \fB\-\-pinned\fR is passed, show the versions of pinned formulae, or only the specified (pinned) formulae if \fIformulae\fR are given\. See also \fBpin\fR, \fBunpin\fR\.
.
.TP
\fBlist\fR

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 8, 2016

Member

would be good to optimise this so the generated output doesn't change at all.

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Apr 8, 2016

This is neat. As a random aside I'd still love to see us use #/ because I've seen it used in multiple other projects, it reads a little nicer to me and I think it would be good to turn off that RuboCop warning anyway

@maxnordlund maxnordlund force-pushed the maxnordlund:brew-update-help branch from ccb4e63 to afa2f54 Apr 8, 2016

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 8, 2016

I've rebased and squashed a bit, and for the rest of the style comments, which I think are mostly good, they can be done in a follow up PR.

maxnordlund added some commits Apr 3, 2016

Implement the `brew help` command
This is also used by `brew <cmd> --help`. The basic idea is to have the
documentation as a top level comment in each command file. To find these
comments, they have to be like this `#:`.

This is also used by the `brew man` command to keep the documentation
DRY, and for that there are now a header and footer for the man page.

@maxnordlund maxnordlund force-pushed the maxnordlund:brew-update-help branch from afa2f54 to 6f8424e Apr 8, 2016

unless help_text.empty?
puts help_text
exit 0
end

This comment has been minimized.

@UniqMartin

UniqMartin Apr 9, 2016

Contributor

This currently creates a regression for undocumented internal commands, e.g., brew help --cache is treated as if --cache was an external command, effectively resulting in brew --cache help. I think this can be addressed by tweaking the output of help_for_command and how it affects what happens here:

  • Non-empty string: Documented internal command; show help text and exit.
  • Empty string: Undocumented internal command; show something like No help available for <command>. followed by generic help and exit.
  • nil: External command; continue execution and let external command handle the help flag.
HOMEBREW_BASH_COMMAND="$HOMEBREW_LIBRARY/Homebrew/cmd/$HOMEBREW_COMMAND.sh"
elif [[ -n "$HOMEBREW_DEVELOPER" && -f "$HOMEBREW_LIBRARY/Homebrew/dev-cmd/$HOMEBREW_COMMAND.sh" ]] ; then
elif [[ -n "$HOMEBREW_DEVELOPER" && -f "$HOMEBREW_LIBRARY/Homebrew/dev-cmd/$HOMEBREW_COMMAND.sh" ]]; then

This comment has been minimized.

@UniqMartin

UniqMartin Apr 9, 2016

Contributor

That's a bit nitpicking, but I'd like to see this unrelated change completely reverted.

@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 9, 2016

I've rebased and squashed a bit, and for the rest of the style comments, which I think are mostly good, they can be done in a follow up PR.

Thanks, looking good! 👍 I'd like to see the regression that I pointed out in a code comment to be addressed, but otherwise I agree. Unless there's a very substantial veto from another maintainer, I'm intending to pull this as soon as the regression is addressed. (The work that has been done here is a significant improvement and I would strongly prefer to defer refinements to follow-up PRs.)

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 9, 2016

I've fixed the regression now, just like @UniqMartin suggested.

@@ -1,3 +1,23 @@
#: * `fetch` [`--force`] [`-v`] [`--devel`|`--HEAD`] [`--deps`] [`--build-from-source`|`--force-bottle`] <formulae>:
#: Download the source packages for the given <formulae>.
#: For tarballs, also print SHA-1 and SHA-256 checksums.

This comment has been minimized.

@DomT4

DomT4 Apr 10, 2016

Contributor

This is no longer true. Only SHA256 is printed now.

This comment has been minimized.

@maxnordlund

maxnordlund Apr 10, 2016

Contributor

Nice, this is from the old man page, good that it gets an look through.

This comment has been minimized.

@DomT4

DomT4 Apr 10, 2016

Contributor

Should be fixed there as well, really. I can do it but don't know if you'd prefer to just wrap that change into this PR?

This comment has been minimized.

@maxnordlund

maxnordlund Apr 10, 2016

Contributor

It should be picked up by the brew man command, but I forgot to run it after this fix. Have done so now.

If there's anything else I guess it's easiest if you just fix it in master.

@maxnordlund maxnordlund force-pushed the maxnordlund:brew-update-help branch from 1ecc206 to b1beaf7 Apr 10, 2016

@UniqMartin

This comment has been minimized.

Copy link
Contributor

UniqMartin commented Apr 10, 2016

Merged in 10edfcd; also took the liberty to make some minor adjustments in 0382134 and to squash your regression fix commit. Thanks a lot for your contribution to Homebrew, @maxnordlund! 🎉

The work you've done here is greatly appreciated and I'm happy you followed up on my suggestion to massively expand the scope of your original PR. Looking forward to see more contributions from you! 😸

@maxnordlund maxnordlund deleted the maxnordlund:brew-update-help branch Apr 10, 2016

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Apr 11, 2016

Great work here @maxnordlund 👏

@maxnordlund

This comment has been minimized.

Copy link
Contributor

maxnordlund commented Apr 11, 2016

Thanks, it's so much nicer to help when you receive some love 😄

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Apr 12, 2016

Just a random comment for here after using it: I think we should delete Library/Homebrew/manpages/brew.1.md and use a different location as the temporary output file. It took me a bit to understand that that file is overwritten when I run brew man and I should be editing e.g. footer.md. It's also not used for anything except input so it feels safe to 🔥

@UniqMartin UniqMartin referenced this pull request Apr 17, 2016

Closed

man: refactor and switch to ERB template #102

4 of 5 tasks complete

@UniqMartin UniqMartin referenced this pull request May 27, 2016

Closed

utils: avoid caching in 'paths' method #283

4 of 5 tasks complete

@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.