From 9daffc70809388f6569f7da724a7c6a469bf3193 Mon Sep 17 00:00:00 2001 From: Aaron Gyes Date: Tue, 16 Aug 2016 15:30:49 -0700 Subject: [PATCH] HeaderDoc code documentation improvements Some cleanup too, move things to builtin.h from builtin.cpp that seem to belong there. --- src/builtin.cpp | 142 ++++++++++++++++++++++++++++------------------- src/builtin.h | 91 +++++++++++++++--------------- src/complete.cpp | 7 +-- src/expand.cpp | 2 +- src/screen.cpp | 4 -- 5 files changed, 131 insertions(+), 115 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index cf205ed48..96f979ec3 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -71,28 +71,6 @@ #include "wgetopt.h" #include "wutil.h" // IWYU pragma: keep -// The default prompt for the read command. -#define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \"" - -// The mode name to pass to history and input. -#define READ_MODE_NAME L"fish_read" - -// The send stuff to foreground message. -#define FG_MSG _(L"Send job %d, '%ls' to foreground\n") - -/// Data structure to describe a builtin. -struct builtin_data_t { - // Name of the builtin. - const wchar_t *name; - // Function pointer tothe builtin implementation. - int (*func)(parser_t &parser, io_streams_t &streams, wchar_t **argv); - // Description of what the builtin does. - const wchar_t *desc; - - bool operator<(const wcstring &) const; - bool operator<(const builtin_data_t *) const; -}; - bool builtin_data_t::operator<(const wcstring &other) const { return wcscmp(this->name, other.c_str()) < 0; } @@ -101,12 +79,18 @@ bool builtin_data_t::operator<(const builtin_data_t *other) const { return wcscmp(this->name, other->name) < 0; } -/// Counts the number of non null pointers in the specified array. +/// Counts the number of arguments in the specified null-terminated array +/// +/// @param argv[]: argument list +/// +/// @return +/// The numer of non-NULL elements in @param *argv before the first NULL. +/// int builtin_count_args(const wchar_t *const *argv) { - int argc = 1; - while (argv[argc] != NULL) { - argc++; - } + int argc; + for (argc = 1; argv[argc] != NULL; argc++); + + assert(argv[argc] == NULL); return argc; } @@ -134,6 +118,14 @@ static int count_char(const wchar_t *str, wchar_t c) { return res; } +/// Obtain help/usage information for the specified builtin from manpage in subshell +/// +/// @param name +/// builtin name to get up help for +/// +/// @return +/// A wcstring with a formatted manpage. +/// wcstring builtin_help_get(parser_t &parser, io_streams_t &streams, const wchar_t *name) { // This won't ever work if no_exec is set. if (no_exec) return wcstring(); @@ -157,10 +149,12 @@ wcstring builtin_help_get(parser_t &parser, io_streams_t &streams, const wchar_t return out; } -/// Print help for the specified builtin. If \c b is sb_err, also print the line information. +/// Process and print for the specified builtin. If @c b is `sb_err`, also print the line +/// information. /// -/// If \c b is the buffer representing standard error, and the help message is about to be printed +/// If @c b is the buffer representing standard error, and the help message is about to be printed /// to an interactive screen, it may be shortened to fit the screen. +/// void builtin_print_help(parser_t &parser, io_streams_t &streams, const wchar_t *cmd, output_stream_t &b) { bool is_stderr = &b == &streams.err; @@ -396,10 +390,16 @@ static int builtin_bind_add(const wchar_t *seq, const wchar_t *const *cmds, size /// Erase specified key bindings /// -/// \param seq an array of all key bindings to erase -/// \param all if specified, _all_ key bindings will be erased -/// \param mode if specified, only bindings from that mode will be erased. If not given and \c all -/// is \c false, \c DEFAULT_BIND_MODE will be used. +/// @param seq +/// an array of all key bindings to erase +/// @param all +/// if specified, _all_ key bindings will be erased +/// @param mode +/// if specified, only bindings from that mode will be erased. If not given +/// and @c all is @c false, @c DEFAULT_BIND_MODE will be used. +/// @param use_terminfo +/// Whether to look use terminfo -k name +/// static int builtin_bind_erase(wchar_t **seq, int all, const wchar_t *mode, int use_terminfo, io_streams_t &streams) { if (all) { @@ -1303,7 +1303,7 @@ static bool builtin_echo_parse_numeric_sequence(const wchar_t *str, size_t *cons /// Bash only respects -n if it's the first argument. We'll do the same. We also support a new /// option -s to mean "no spaces" static int builtin_echo(parser_t &parser, io_streams_t &streams, wchar_t **argv) { - /* Skip first arg */ + // Skip first arg if (!*argv++) return STATUS_BUILTIN_ERROR; // Process options. Options must come at the beginning - the first non-option kicks us out. @@ -1464,7 +1464,20 @@ static int builtin_pwd(parser_t &parser, io_streams_t &streams, wchar_t **argv) return STATUS_BUILTIN_OK; } -/// Adds a function to the function set. It calls into function.cpp to perform any heavy lifting. +/// Defines and adds a function to the function set. Calls into `function.cpp` +/// to perform all heavy lifting. +/// +/// @param c_args +/// The arguments. Should NOT contain 'function' as the first argument as the +/// parser treats it as a keyword. +/// @param contents +/// The function definition string +/// @param definition_line_offset +/// The definition line offset +/// +/// @return +/// Returns 0 on success. +/// int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_list_t &c_args, const wcstring &contents, int definition_line_offset, wcstring *out_err) { wgetopter_t w; @@ -2097,7 +2110,7 @@ static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) } if (buff.empty() && eof) { - exit_res = 1; + exit_res = STATUS_BUILTIN_ERROR; } } @@ -2119,7 +2132,7 @@ static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) } else { env_set(argv[i], NULL, place); } - } else { + } else { // not array size_t j = 0; for (; i + 1 < argc; ++i) { if (j < bufflen) { @@ -2143,14 +2156,13 @@ static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) empty = false; } env_set(argv[i], empty ? NULL : tokens.c_str(), place); - } else { + } else { // not array wcstring_range loc = wcstring_range(0, 0); while (i < argc) { loc = wcstring_tok(buff, (i + 1 < argc) ? ifs : wcstring(), loc); env_set(argv[i], loc.first == wcstring::npos ? L"" : &buff.c_str()[loc.first], place); - ++i; } } @@ -2367,7 +2379,6 @@ static int builtin_exit(parser_t &parser, io_streams_t &streams, wchar_t **argv) static int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { env_var_t dir_in; wcstring dir; - int res = STATUS_BUILTIN_OK; if (argv[1] == NULL) { dir_in = env_get_string(L"HOME"); @@ -2403,15 +2414,16 @@ static int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { streams.err.append(parser.current_line()); } - res = 1; - } else if (wchdir(dir) != 0) { + return STATUS_BUILTIN_ERROR; + } + + if (wchdir(dir) != 0) { struct stat buffer; int status; status = wstat(dir, &buffer); if (!status && S_ISDIR(buffer.st_mode)) { streams.err.append_format(_(L"%ls: Permission denied: '%ls'\n"), argv[0], dir.c_str()); - } else { streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), argv[0], dir.c_str()); } @@ -2420,13 +2432,14 @@ static int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { streams.err.append(parser.current_line()); } - res = 1; - } else if (!env_set_pwd()) { - res = 1; - streams.err.append_format(_(L"%ls: Could not set PWD variable\n"), argv[0]); + return STATUS_BUILTIN_ERROR; } - return res; + if (!env_set_pwd()) { + streams.err.append_format(_(L"%ls: Could not set PWD variable\n"), argv[0]); + return STATUS_BUILTIN_ERROR; + } else + return STATUS_BUILTIN_OK; } /// Implementation of the builtin count command, used to count the number of arguments sent to it. @@ -3079,7 +3092,7 @@ int builtin_fish_realpath(parser_t &parser, io_streams_t &streams, wchar_t **arg int argc = builtin_count_args(argv); if (argc != 2) { - streams.err.append_format(_(L"%ls: Expected one argument, got %d\n"), argv[0], argc - 1); + builtin_print_help(parser, streams, argv[0], streams.out); return STATUS_BUILTIN_ERROR; } @@ -3089,7 +3102,7 @@ int builtin_fish_realpath(parser_t &parser, io_streams_t &streams, wchar_t **arg streams.out.append(real_path); free((void *)real_path); } else { - // The path isn't a simple filename and couldn't be resolved to an absolute path. + // We don't actually know why it failed. We should check errno streams.err.append_format(_(L"%ls: Invalid path: %ls\n"), argv[0], argv[1]); return STATUS_BUILTIN_ERROR; } @@ -3164,6 +3177,14 @@ static const builtin_data_t builtin_datas[] = { #define BUILTIN_COUNT (sizeof builtin_datas / sizeof *builtin_datas) +/// Look up a builtin_data_t for a specified builtin +/// +/// @param name +/// Name of the builtin +/// +/// @return +/// Pointer to a builtin_data_t +/// static const builtin_data_t *builtin_lookup(const wcstring &name) { const builtin_data_t *array_end = builtin_datas + BUILTIN_COUNT; const builtin_data_t *found = std::lower_bound(builtin_datas, array_end, name); @@ -3173,35 +3194,37 @@ static const builtin_data_t *builtin_lookup(const wcstring &name) { return NULL; } +/// Initialize builtin data. void builtin_init() { for (size_t i = 0; i < BUILTIN_COUNT; i++) { intern_static(builtin_datas[i].name); } } +/// Destroy builtin data. void builtin_destroy() {} -int builtin_exists(const wcstring &cmd) { return !!builtin_lookup(cmd); } +/// Is there a builtin command with the given name? +bool builtin_exists(const wcstring &cmd) { return !!builtin_lookup(cmd); } -/// Return true if the specified builtin should handle it's own help, false otherwise. -static int internal_help(const wchar_t *cmd) { +/// If builtin takes care of printing help itself +static bool builtin_handles_help(const wchar_t *cmd) { CHECK(cmd, 0); return contains(cmd, L"for", L"while", L"function", L"if", L"end", L"switch", L"case", L"count", L"printf"); } +/// Execute a builtin command int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &streams) { int (*cmd)(parser_t & parser, io_streams_t & streams, const wchar_t *const *argv) = NULL; - - CHECK(argv, STATUS_BUILTIN_ERROR); - CHECK(argv[0], STATUS_BUILTIN_ERROR); + if (argv == NULL || argv[0] == NULL) return STATUS_BUILTIN_ERROR; const builtin_data_t *data = builtin_lookup(argv[0]); cmd = (int (*)(parser_t & parser, io_streams_t & streams, const wchar_t *const *))( data ? data->func : NULL); - if (argv[1] != 0 && !internal_help(argv[0])) { - if (argv[2] == 0 && (parse_util_argument_is_help(argv[1], 0))) { + if (argv[1] != NULL && !builtin_handles_help(argv[0])) { + if (argv[2] == NULL && (parse_util_argument_is_help(argv[1], 0))) { builtin_print_help(parser, streams, argv[0], streams.out); return STATUS_BUILTIN_OK; } @@ -3215,6 +3238,7 @@ int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &stre return STATUS_BUILTIN_ERROR; } +/// Returns a list of all builtin names. wcstring_list_t builtin_get_names(void) { wcstring_list_t result; result.reserve(BUILTIN_COUNT); @@ -3224,6 +3248,7 @@ wcstring_list_t builtin_get_names(void) { return result; } +/// Insert all builtin names into list. void builtin_get_names(std::vector *list) { assert(list != NULL); list->reserve(list->size() + BUILTIN_COUNT); @@ -3232,6 +3257,7 @@ void builtin_get_names(std::vector *list) { } } +/// Return a one-line description of the specified builtin. wcstring builtin_get_desc(const wcstring &name) { wcstring result; const builtin_data_t *builtin = builtin_lookup(name); diff --git a/src/builtin.h b/src/builtin.h index 62ce63657..45c0b0f9a 100644 --- a/src/builtin.h +++ b/src/builtin.h @@ -12,79 +12,86 @@ class parser_t; class output_stream_t; struct io_streams_t; +/// Data structure to describe a builtin. +struct builtin_data_t { + // Name of the builtin. + const wchar_t *name; + // Function pointer tothe builtin implementation. + int (*func)(parser_t &parser, io_streams_t &streams, wchar_t **argv); + // Description of what the builtin does. + const wchar_t *desc; + + bool operator<(const wcstring &) const; + bool operator<(const builtin_data_t *) const; +}; + +/// The default prompt for the read command. +#define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \"" + +/// The mode name to pass to history and input. +#define READ_MODE_NAME L"fish_read" + enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION }; -// Error message on missing argument. +/// Error message on missing argument. #define BUILTIN_ERR_MISSING _(L"%ls: Expected argument for option %ls\n") -// Error message on invalid combination of options. +/// Error message on invalid combination of options. #define BUILTIN_ERR_COMBO _(L"%ls: Invalid combination of options\n") -// Error message on invalid combination of options. +/// Error message on invalid combination of options. #define BUILTIN_ERR_COMBO2 _(L"%ls: Invalid combination of options,\n%ls\n") -// Error message on multiple scope levels for variables. +/// Error message on multiple scope levels for variables. #define BUILTIN_ERR_GLOCAL \ _(L"%ls: Variable scope can only be one of universal, global and local\n") -// Error message for specifying both export and unexport to set/read. +/// Error message for specifying both export and unexport to set/read. #define BUILTIN_ERR_EXPUNEXP _(L"%ls: Variable can't be both exported and unexported\n") -// Error message for unknown switch. +/// Error message for unknown switch. #define BUILTIN_ERR_UNKNOWN _(L"%ls: Unknown option '%ls'\n") -// Error message for unexpected args. +/// Error message for unexpected args. #define BUILTIN_ERR_ARG_COUNT _(L"%ls: %ls command expected %d args, got %d\n") -// Error message for invalid character in variable name. +/// Error message for invalid character in variable name. #define BUILTIN_ERR_VARCHAR \ _(L"%ls: Invalid character '%lc' in variable name. Only alphanumerical characters and " \ L"underscores are valid in a variable name.\n") -// Error message for invalid (empty) variable name. +/// Error message for invalid (empty) variable name. #define BUILTIN_ERR_VARNAME_ZERO _(L"%ls: Variable name can not be the empty string\n") -// Error message when too many arguments are supplied to a builtin. +/// Error message when too many arguments are supplied to a builtin. #define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: Too many arguments\n") +/// Error message when number expected #define BUILTIN_ERR_NOT_NUMBER _(L"%ls: Argument '%ls' is not a number\n") -// Initialize builtin data. +/// The send stuff to foreground message. +#define FG_MSG _(L"Send job %d, '%ls' to foreground\n") + void builtin_init(); - -// Destroy builtin data. void builtin_destroy(); +bool builtin_exists(const wcstring &cmd); -// Is there a builtin command with the given name? -int builtin_exists(const wcstring &cmd); - -// Execute a builtin command -// -// \param parser The parser being used -// \param argv Array containing the command and parameters of the builtin. The list is terminated -// by a null pointer. This syntax resembles the syntax for exec. -// \param io the io redirections to perform on this builtin. -// -// \return the exit status of the builtin command int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &streams); -// Returns a list of all builtin names. wcstring_list_t builtin_get_names(); - -// Insert all builtin names into list. void builtin_get_names(std::vector *list); - -// Return a one-line description of the specified builtin. wcstring builtin_get_desc(const wcstring &b); -// Support for setting and removing transient command lines. This is used by 'complete -C' in order -// to make the commandline builtin operate on the string to complete instead of operating on -// whatever is to be completed. It's also used by completion wrappers, to allow a command to appear -// as the command being wrapped for the purposes of completion. -// -// Instantiating an instance of builtin_commandline_scoped_transient_t pushes the command as the new -// transient commandline. The destructor removes it. It will assert if construction/destruction does -// not happen in a stack-like (LIFO) order. +/// Support for setting and removing transient command lines. This is used by +/// 'complete -C' in order to make the commandline builtin operate on the string +/// to complete instead of operating on whatever is to be completed. It's also +/// used by completion wrappers, to allow a command to appear as the command +/// being wrapped for the purposes of completion. +/// +/// Instantiating an instance of builtin_commandline_scoped_transient_t pushes +/// the command as the new transient commandline. The destructor removes it. It +/// will assert if construction/destruction does not happen in a stack-like +/// (LIFO) order. class builtin_commandline_scoped_transient_t { size_t token; @@ -93,30 +100,20 @@ class builtin_commandline_scoped_transient_t { ~builtin_commandline_scoped_transient_t(); }; -// Run the __fish_print_help function to obtain the help information for the specified command. wcstring builtin_help_get(parser_t &parser, const wchar_t *cmd); -// Defines a function. Returns 0 on success. args should NOT contain 'function' as the first -// argument as the parser treats it as a keyword. int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_list_t &c_args, const wcstring &contents, int definition_line_offset, wcstring *out_err); -// Print help for the specified builtin. If \c b is sb_err, also print the line information. void builtin_print_help(parser_t &parser, io_streams_t &streams, const wchar_t *cmd, output_stream_t &b); - -// Counts the number of non null pointers in the specified array. int builtin_count_args(const wchar_t *const *argv); -// Perform error reporting for encounter with unknown option. void builtin_unknown_option(parser_t &parser, io_streams_t &streams, const wchar_t *cmd, const wchar_t *opt); -// Perform error reporting for encounter with missing argument. void builtin_missing_argument(parser_t &parser, io_streams_t &streams, const wchar_t *cmd, const wchar_t *opt); -// This function works like wperror, but it prints its result into the streams.err string instead -// to stderr. Used by the builtin commands. void builtin_wperror(const wchar_t *s, io_streams_t &streams); #endif diff --git a/src/complete.cpp b/src/complete.cpp index cec5ac19e..2bb8f78f0 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -123,7 +123,6 @@ typedef struct complete_entry_opt { } } complete_entry_opt_t; - // Last value used in the order field of completion_entry_t. static unsigned int kCompleteOrder = 0; @@ -375,7 +374,6 @@ void append_completion(std::vector *completions, const wcstring &c // Nasty hack for #1241 - since the constructor needs the completion string to resolve // AUTO_SPACE, and we aren't providing it with the completion, we have to do the resolution // ourselves. We should get this resolving out of the constructor. - assert(completions != NULL); const wcstring empty; completions->push_back(completion_t(empty, empty, match, resolve_auto_space(comp, flags))); completion_t *last = &completions->back(); @@ -821,7 +819,7 @@ static bool short_ok(const wcstring &arg, const complete_entry_opt_t *entry, return result; } -// Load command-specific completions for the specified command. +/// Load command-specific completions for the specified command. static void complete_load(const wcstring &name, bool reload) { // We have to load this as a function, since it may define a --wraps or signature. // See issue #2466. @@ -829,9 +827,8 @@ static void complete_load(const wcstring &name, bool reload) { completion_autoloader.load(name, reload); } -// Performed on main thread, from background thread. Return type is ignored. +/// Performed on main thread, from background thread. Return type is ignored. static int complete_load_no_reload(wcstring *name) { - assert(name != NULL); ASSERT_IS_MAIN_THREAD(); complete_load(*name, false); return 0; diff --git a/src/expand.cpp b/src/expand.cpp index 5ea21a144..1b80fd334 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -1603,7 +1603,7 @@ bool expand_abbreviation(const wcstring &src, wcstring *output) { // See if this command matches. if (line.compare(0, cmd_end, src) == 0) { - // Success. Set output to everythign past the end of the string. + // Success. Set output to everything past the end of the string. if (output != NULL) output->assign(line, separator + 1, wcstring::npos); result = true; diff --git a/src/screen.cpp b/src/screen.cpp index 5bd6de1f3..c4159e36a 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -223,9 +223,7 @@ size_t escape_code_length(const wchar_t *code) { } } } - } - if (cur_term != NULL) { // Detect these semi-common terminfo escapes without any parameter values, all of which // don't move the cursor. char *const esc2[] = {enter_bold_mode, exit_attribute_mode, enter_underline_mode, @@ -1039,7 +1037,6 @@ void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_pro const int *indent, size_t cursor_pos, const page_rendering_t &pager, bool cursor_is_within_pager) { screen_data_t::cursor_t cursor_arr; - CHECK(s, ); CHECK(indent, ); @@ -1126,7 +1123,6 @@ void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_pro s_update(s, layout.left_prompt.c_str(), layout.right_prompt.c_str()); s_save_status(s); } - void s_reset(screen_t *s, screen_reset_mode_t mode) { CHECK(s, );