On 10 May 2007, at 02:01, Eric Bergen wrote:
> One of the many things that I learned at the user conference is the
> correct way to send patches (thanks Jay!).
Hi again! I should mention you also learned about the "#mysql-dev"
IRC channel on Freenode, which is dedicated to development of MySQL
code.
> This email is a posting of
> mysql bug http://bugs.mysql.com/bug.php?id=26780 which is my patch to
> add --auto-vertical-output to the client. This option compares the
> width of the result set to the width of the terminal and enables
> vertical display if the result is wider than the terminal window. This
> saves rerunning a query with \G when the results are wrapped on the
> terminal.
>
> The patch applies against 5.0.41
>
> The patch is at http://ebergen.net/patches/auto_vertical.patch
> diff -urN mysql-5.0.41-orig/client/client_priv.h mysql-5.0.41/
> client/client_priv.h
> --- mysql-5.0.41-orig/client/client_priv.h 2007-05-09
> 22:13:55.000000000 -0700
> +++ mysql-5.0.41/client/client_priv.h 2007-05-09 22:49:48.000000000
> -0700
> @@ -50,6 +50,6 @@
> #endif
> OPT_TRIGGERS,
>
> OPT_IGNORE_TABLE,OPT_INSERT_IGNORE,OPT_SHOW_WARNINGS,OPT_DROP_DATABASE
> ,
> - OPT_TZ_UTC, OPT_AUTO_CLOSE, OPT_SSL_VERIFY_SERVER_CERT,
> + OPT_TZ_UTC, OPT_AUTO_CLOSE, OPT_AUTO_VERTICAL_OUTPUT,
> OPT_SSL_VERIFY_SERVER_CERT,
This is minor: They're not in alphabetical order, so adding
OPT_AUTO_VERTICAL_OUTPUT on a line by itself would not cause a line
to be removed and added, thus perhaps saving a merge conflict.
+ OPT_AUTO_VERTICAL_OUTPUT,
> OPT_DEBUG_INFO
> };
> diff -urN mysql-5.0.41-orig/client/mysql.cc mysql-5.0.41/client/
> mysql.cc
> --- mysql-5.0.41-orig/client/mysql.cc 2007-05-09 22:13:55.000000000
> -0700
> +++ mysql-5.0.41/client/mysql.cc 2007-05-09 22:57:42.000000000 -0700
> @@ -137,7 +137,7 @@
> opt_xml=0,opt_nopager=1, opt_outfile=0, named_cmds= 0,
> tty_password= 0, opt_nobeep=0, opt_reconnect=1,
> default_charset_used= 0, opt_secure_auth= 0,
> - default_pager_set= 0, opt_sigint_ignore= 0,
> + default_pager_set= 0, opt_sigint_ignore= 0,
> auto_vertical_output= 0,
Again here. It's a tight path we walk between keeping code beautiful
and making small patches. In this case, it's probably better to go
the latter way.
> show_warnings= 0;
> static volatile int executing_query= 0, interrupted_query= 0;
> static ulong opt_max_allowed_packet, opt_net_buffer_length;
> @@ -173,6 +173,7 @@
> static uint prompt_counter;
> static char delimiter[16]= DEFAULT_DELIMITER;
> static uint delimiter_length= 1;
> +unsigned short terminal_width= 80;
Yes, good!
>
> #ifdef HAVE_SMEM
> static char *shared_memory_base_name=0;
> @@ -224,6 +225,8 @@
> static char *get_arg(char *line, my_bool get_next_arg);
> static void init_username();
> static void add_int_to_prompt(int toadd);
> +static int get_result_width(MYSQL_RES *res);
> +static int get_field_disp_length(MYSQL_FIELD * field);
>
> /* A structure which contains information on the commands this
> program
> can understand. */
> @@ -343,7 +346,9 @@
> static void nice_time(double sec,char *buff,bool part_second);
> static sig_handler mysql_end(int sig);
> static sig_handler mysql_sigint(int sig);
> -
> +#if defined(HAVE_TERMIOS_H)
> +static sig_handler window_resize(int sig);
> +#endif
>
> int main(int argc,char *argv[])
> {
> @@ -430,8 +435,8 @@
> if (sql_connect(current_host,current_db,current_user,opt_password,
> opt_silent))
> {
> - quick=1; // Avoid history
> - status.exit_status=1;
> + quick= 1; // Avoid history
> + status.exit_status= 1;
Thank you.
> mysql_end(-1);
> }
> if (!status.batch)
> @@ -443,6 +448,13 @@
> signal(SIGINT, mysql_sigint); // Catch SIGINT to clean up
> signal(SIGQUIT, mysql_end); // Catch SIGQUIT to clean up
>
> +#if defined(HAVE_TERMIOS_H)
> + //Readline will call this if it installs a handler
Some C compilers we support don't implement C++/C99 slashslash-comments.
> + signal(SIGWINCH, window_resize);
> + //call the SIGWINCH handler to get the default term width
> + window_resize(0);
> +#endif
> +
> put_info("Welcome to the MySQL monitor. Commands end with ; or \
> \g.",
> INFO_INFO);
> sprintf((char*) glob_buffer.ptr(),
> @@ -569,6 +581,16 @@
> }
>
>
> +#if defined(HAVE_TERMIOS_H)
> +sig_handler window_resize(int sig)
> +{
> + struct winsize window_size;
> +
> + if (!ioctl(fileno(stdin), TIOCGWINSZ, &window_size))
I'd rather have "if (ioctl() == 0)". "Not" feels bad when testing
for success.
> + terminal_width = window_size.ws_col;
The spaces around the equal sign aren't right, according to our style
guide.
> +}
> +#endif
> +
> static struct my_option my_long_options[] =
> {
> {"help", '?', "Display this help and exit.", 0, 0, 0,
> GET_NO_ARG, NO_ARG, 0,
> @@ -586,6 +608,9 @@
> {"no-auto-rehash", 'A',
> "No automatic rehashing. One has to use 'rehash' to get table
> and field completion. This gives a quicker start of mysql and
> disables rehashing on reconnect. WARNING: options deprecated; use --
> disable-auto-rehash instead.",
> 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
> + {"auto-vertical-output", OPT_AUTO_VERTICAL_OUTPUT,
> + "Automatically switch to vertical output mode if the result is
> wider than the terminal width.",
> + (gptr*) &auto_vertical_output, (gptr*) &auto_vertical_output,
> 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
> {"batch", 'B',
> "Don't use history file. Disable interactive behavior. (Enables
> --silent)", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
> {"character-sets-dir", OPT_CHARSETS_DIR,
> @@ -2125,7 +2150,7 @@
> print_table_data_html(result);
> else if (opt_xml)
> print_table_data_xml(result);
> - else if (vertical)
> + else if (vertical || (auto_vertical_output && terminal_width <
> get_result_width(result)))
I think extra parens around t_w < g_r_w(r) is a good idea, for
legibility.
> print_table_data_vertically(result);
> else if (opt_silent && verbose <= 2 && !output_tables)
> print_tab_data(result);
> @@ -2454,6 +2479,33 @@
> my_afree((gptr) num_flag);
> }
>
> +static int get_field_disp_length(MYSQL_FIELD * field)
The space after the asterisk isn't like the rest of the code, right?
> +{
> + uint length= column_names ? field->name_length : 0;
> +
> + if (quick)
> + length=max(length,field->length);
> + else
> + length=max(length,field->max_length);
variable, equal-sign, one space, value.
> +
> + if (length < 4 && !IS_NOT_NULL(field->flags))
> + length= 4; // Room for "NULL"
> +
> + return length + 1;
There's some extra space after these lines, I notice.
What's the extra "1" for? That deserves a comment.
> +}
> +
> +static int get_result_width(MYSQL_RES *result)
> +{
> + unsigned int len= 0;
> + MYSQL_FIELD *field;
> +
> + while ((field = mysql_fetch_field(result)) != NULL)
Extraneous space.
> + len+= get_field_disp_length(field) + 2;
> +
> + mysql_field_seek(result, 0);
A "(void)" at the beginning emphasizes we don't care about the return
value.
> +
> + return len + 1;
> +}
>
> static void
> tee_print_sized_data(const char *data, unsigned int data_length,
> unsigned int total_bytes_to_send, bool right_justified)
A nice patch, and great idea. With a little cleanup, I'd gladly add
it to the tree.
- chad
--
Chad Miller, Software Developer chad@stripped
MySQL Inc., www.mysql.com
Orlando, Florida, USA 13-20z, UTC-0400
Office: +1 408 213 6740 sip:6740@stripped
Attachment: [application/pgp-signature] This is a digitally signed message part PGP.sig
