Skip to content

Commit 11b2af0

Browse files
committed
Remove dead code from display subcommand handler
The display subcommand was set up to be intercepted when given the help flag, so the code to call the internal binary only ran when calling it with no further arguments. Given that the internal binary's help message and the subcommand's are nearly identical it's better to always print its own help without deferring to the internal one.
1 parent b01845a commit 11b2af0

2 files changed

Lines changed: 9 additions & 41 deletions

File tree

base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,9 @@ cf_cc_library(
9292
srcs = ["display.cpp"],
9393
hdrs = ["display.h"],
9494
deps = [
95-
"//cuttlefish/common/libs/utils:contains",
9695
"//cuttlefish/common/libs/utils:files",
9796
"//cuttlefish/common/libs/utils:flag_parser",
9897
"//cuttlefish/common/libs/utils:subprocess",
99-
"//cuttlefish/common/libs/utils:users",
10098
"//cuttlefish/host/commands/cvd/cli:command_request",
10199
"//cuttlefish/host/commands/cvd/cli:types",
102100
"//cuttlefish/host/commands/cvd/cli:utils",

base/cvd/cuttlefish/host/commands/cvd/cli/commands/display.cpp

Lines changed: 9 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,9 @@
2828

2929
#include "absl/strings/str_cat.h"
3030

31-
#include "cuttlefish/common/libs/utils/contains.h"
3231
#include "cuttlefish/common/libs/utils/files.h"
3332
#include "cuttlefish/common/libs/utils/flag_parser.h"
3433
#include "cuttlefish/common/libs/utils/subprocess.h"
35-
#include "cuttlefish/common/libs/utils/users.h"
3634
#include "cuttlefish/host/commands/cvd/cli/command_request.h"
3735
#include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h"
3836
#include "cuttlefish/host/commands/cvd/cli/selector/selector.h"
@@ -71,12 +69,14 @@ class CvdDisplayCommandHandler : public CvdCommandHandler {
7169
const cvd_common::Envs& env = request.Env();
7270

7371
std::vector<std::string> subcmd_args = request.SubcommandArguments();
72+
if (subcmd_args.empty()) {
73+
// Also print help when given no arguments
74+
std::cerr << kDetailedHelpText;
75+
return {};
76+
}
7477

75-
bool is_help = CF_EXPECT(IsHelp(subcmd_args));
7678
// may modify subcmd_args by consuming in parsing
77-
Command command =
78-
is_help ? CF_EXPECT(HelpCommand(request, subcmd_args, env))
79-
: CF_EXPECT(NonHelpCommand(request, subcmd_args, env));
79+
Command command = CF_EXPECT(BuildCommand(request, subcmd_args, env));
8080

8181
siginfo_t infop; // NOLINT(misc-include-cleaner)
8282
command.Start().Wait(&infop, WEXITED);
@@ -98,30 +98,9 @@ class CvdDisplayCommandHandler : public CvdCommandHandler {
9898
}
9999

100100
private:
101-
Result<Command> HelpCommand(const CommandRequest& request,
102-
const cvd_common::Args& subcmd_args,
103-
cvd_common::Envs envs) {
104-
const std::string android_host_out = CF_EXPECT(AndroidHostPath(envs));
105-
const std::string cvd_display_bin_path =
106-
absl::StrCat(android_host_out, "/bin/", kDisplayBin);
107-
std::string home = Contains(envs, "HOME") ? envs.at("HOME")
108-
: CF_EXPECT(SystemWideUserHome());
109-
envs["HOME"] = home;
110-
envs[kAndroidHostOut] = android_host_out;
111-
envs[kAndroidSoongHostOut] = android_host_out;
112-
ConstructCommandParam construct_cmd_param{.bin_path = cvd_display_bin_path,
113-
.home = home,
114-
.args = subcmd_args,
115-
.envs = std::move(envs),
116-
.working_dir = CurrentDirectory(),
117-
.command_name = kDisplayBin};
118-
Command command = CF_EXPECT(ConstructCommand(construct_cmd_param));
119-
return command;
120-
}
121-
122-
Result<Command> NonHelpCommand(const CommandRequest& request,
123-
cvd_common::Args& subcmd_args,
124-
cvd_common::Envs envs) {
101+
Result<Command> BuildCommand(const CommandRequest& request,
102+
cvd_common::Args& subcmd_args,
103+
cvd_common::Envs envs) {
125104
// test if there is --instance_num flag
126105
int instance_num = -1;
127106
Flag instance_num_flag = GflagsCompatFlag("instance_num", instance_num);
@@ -163,15 +142,6 @@ class CvdDisplayCommandHandler : public CvdCommandHandler {
163142
return command;
164143
}
165144

166-
Result<bool> IsHelp(const cvd_common::Args& cmd_args) const {
167-
// cvd display --help, --helpxml, etc or simply cvd display
168-
if (cmd_args.empty() || CF_EXPECT(HasHelpFlag(cmd_args))) {
169-
return true;
170-
}
171-
// cvd display help <subcommand> format
172-
return (cmd_args.front() == "help");
173-
}
174-
175145
InstanceManager& instance_manager_;
176146
static constexpr char kDisplayBin[] = "cvd_internal_display";
177147
};

0 commit comments

Comments
 (0)