Re: [PATCH 3/4] sparse-checkout: create 'add' subcommand

"Derrick Stolee via GitGitGadget" [off-list ref] writes:

+	switch (m) {
+	case ADD:
+		if (core_sparse_checkout_cone)
+			add_patterns_cone_mode(argc, argv, &pl);
+		else
+			add_patterns_literal(argc, argv, &pl);
+		break;
+
+	case REPLACE:
+		add_patterns_from_input(&pl, argc, argv);
+		break;
+	}

Is it just me or do readers find it irritating to see the order of
the arguments seem a bit random?  Those who like Pseudo-OO-in-C
probably would want to consistently see &pl as the first parameter,
which lets them pretend various flavours of add_patterns_*() are
all "methods" to the pattern list object.

+static void add_patterns_literal(int argc, const char **argv,
+				 struct pattern_list *pl)
+{
+	char *sparse_filename = get_sparse_checkout_filename();
+	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
+					   pl, NULL))
+		die(_("unable to load existing sparse-checkout patterns"));
+	free(sparse_filename);
+	add_patterns_from_input(pl, argc, argv);
+}

It may make it easier to read the caller if this helper were
replaced with one that does not add anything but just read from the
existing file, i.e.

		if (core_sparse_checkout_cone)
			add_patterns_cone_mode(argc, argv, &pl);
		else {
			read_existing_patterns(&pl);
			add_patterns_from_input(&pl, argc, argv);
		}