MySQLTalk.org Forum Index MySQLTalk.org
MYSQL discussions groups
 
Archives   FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

Auto vertical output feature patch

 
Post new topic   Reply to topic    MySQLTalk.org Forum Index -> MySQL Internals
View previous topic :: View next topic  
Author Message
Eric Bergen
Guest





PostPosted: Thu May 10, 2007 10:11 am    Post subject: Auto vertical output feature patch Reply with quote



One of the many things that I learned at the user conference is the
correct way to send patches (thanks Jay!). 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

--
high performance mysql consulting.
http://provenscaling.com
Back to top
Jay Pipes
Guest





PostPosted: Thu May 10, 2007 9:33 pm    Post subject: Re: Auto vertical output feature patch Reply with quote



Eric Bergen wrote:
Quote:
One of the many things that I learned at the user conference is the
correct way to send patches (thanks Jay!). 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.

Like I said at the UC... wicked cool. :)

Quote:
The patch applies against 5.0.41

The patch is at http://ebergen.net/patches/auto_vertical.patch

Excellent. We've already received a CLA from you I believe, so I think
everything is cool to begin deciding on what version this might go into.
We have a backlog on features that we are currently evaluating which
version they should go into and, generally, the process/policy by which
those decisions are made. So, be patient, and know we are actively
working towards getting features into the server in a timely fashion.

Manu, I've cc'd you too, since your patch for heap engine blobs is one
of those currently in the waiting pattern as we decide on this policy.

Cheers, and thanks much to you both!

--
Jay Pipes
Community Relations Manager, North America, MySQL
jay (AT) mysql (DOT) com
Back to top
Chad MILLER
Guest





PostPosted: Tue May 15, 2007 8:56 pm    Post subject: IRC channel for development, and patch review, was Re: Auto Reply with quote



On 10 May 2007, at 02:01, Eric Bergen wrote:

Quote:
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.

Quote:
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


Quote:
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,


Quote:
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.

Quote:
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!

Quote:

#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.

Quote:
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.

Quote:
+ 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.

Quote:
+ terminal_width = window_size.ws_col;

The spaces around the equal sign aren't right, according to our style
guide.

Quote:
+}
+#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.

Quote:
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?

Quote:
+{
+ 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.

Quote:
+
+ 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.

Quote:
+}
+
+static int get_result_width(MYSQL_RES *result)
+{
+ unsigned int len= 0;
+ MYSQL_FIELD *field;
+
+ while ((field = mysql_fetch_field(result)) != NULL)

Extraneous space.

Quote:
+ 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.

Quote:
+
+ 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 (AT) mysql (DOT) com
MySQL Inc., www.mysql.com
Orlando, Florida, USA 13-20z, UTC-0400
Office: +1 408 213 6740 sip:6740 (AT) sip (DOT) mysql.com
Back to top
Eric Bergen
Guest





PostPosted: Tue May 22, 2007 12:59 am    Post subject: Re: IRC channel for development, and patch review, was Re: A Reply with quote

Chad,

http://ebergen.net/patches/auto_vertical.patch has been updated per
your requirements.

Regards,
Eric

On 5/15/07, Chad MILLER <cmiller (AT) mysql (DOT) com> wrote:
Quote:
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 (AT) mysql (DOT) com
MySQL Inc., www.mysql.com
Orlando, Florida, USA 13-20z, UTC-0400
Office: +1 408 213 6740 sip:6740 (AT) sip (DOT) mysql.com






--
high performance mysql consulting.
http://provenscaling.com
Back to top
Display posts from previous:   
Post new topic   Reply to topic    MySQLTalk.org Forum Index -> MySQL Internals All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2006 phpBB Group
SEO toolkit © 2004-2006 webmedic.