Re: [PATCH] Style "pwd" output

[Pedro Alves <palves at redhat dot com>]

On 6/5/19 3:01 AM, Tom Tromey wrote:
> This changes the "pwd" command to style its output.
> Tested on x86-64 Fedora 29.
> 
> gdb/ChangeLog
> 2019-06-04  Tom Tromey  <tom@tromey.com>
> 
> 	* cli/cli-cmds.c (pwd_command): Style output.
> 
> gdb/testsuite/ChangeLog
> 2019-06-04  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.base/style.exp: Test "pwd".
> ---
>  gdb/ChangeLog                    |  4 ++++
>  gdb/cli/cli-cmds.c               | 14 +++++++++-----
>  gdb/testsuite/ChangeLog          |  4 ++++
>  gdb/testsuite/gdb.base/style.exp |  2 ++
>  4 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 09f932c2d21..658b08e49a6 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -49,6 +49,7 @@
>  #include "cli/cli-script.h"
>  #include "cli/cli-setshow.h"
>  #include "cli/cli-cmds.h"
> +#include "cli/cli-style.h"
>  #include "cli/cli-utils.h"
>  
>  #include "extension.h"
> @@ -337,11 +338,14 @@ pwd_command (const char *args, int from_tty)
>      error (_("Error finding name of working directory: %s"),
>             safe_strerror (errno));
>  
> -  if (strcmp (cwd.get (), current_directory) != 0)
> -    printf_unfiltered (_("Working directory %s\n (canonically %s).\n"),
> -		       current_directory, cwd.get ());
> -  else
> -    printf_unfiltered (_("Working directory %s.\n"), current_directory);
> +  fputs_filtered (_("Working directory "), gdb_stdout);
> +  fputs_styled (current_directory, file_name_style.style (), gdb_stdout);
> +  if (strcmp (cwd.get (), current_directory) == 0)
> +    {
> +      fputs_filtered (_("\n (canonically "), gdb_stdout);
> +      fputs_styled (cwd.get (), file_name_style.style (), gdb_stdout);
> +    }
> +  fputs_filtered (".\n", gdb_stdout);

Seems fine to me.

I wish we didn't have to split the lines across different calls though.
I know we don't officially do i18n yet (*), but these kinds of changes will
only make it more difficult to get there.

Maybe with something like:

  if (strcmp (cwd.get (), current_directory) != 0)
    printf_unfiltered (_("Working directory <style=filename>%s<style/>\n (canonically <style=filename>%s<style/>).\n"),
		       current_directory, cwd.get ());
  else
    printf_unfiltered (_("Working directory <style=filename>%s<style/>.\n"), current_directory);

I'm sure you've considered something like that; I think we've discussed it
before.  What are your current thoughts?

(*) - I don't think there's any real blocker other than someone
      figuring out the process.  We should just do with that binutils
      folks do, I suppose.

Thanks,
Pedro Alves