From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99289 invoked by alias); 9 Sep 2017 17:47:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 99260 invoked by uid 89); 9 Sep 2017 17:47:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 09 Sep 2017 17:47:12 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v89Hl58f030519 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 9 Sep 2017 13:47:10 -0400 Received: by simark.ca (Postfix, from userid 112) id 4EC391EAAB; Sat, 9 Sep 2017 13:47:05 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id B3C0E1EA18; Sat, 9 Sep 2017 13:47:04 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 09 Sep 2017 17:47:00 -0000 From: Simon Marchi To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFA 1/7] Use ui_out_emit_table and ui_out_emit_list in print_thread_info_1 In-Reply-To: <20170909153540.15008-2-tom@tromey.com> References: <20170909153540.15008-1-tom@tromey.com> <20170909153540.15008-2-tom@tromey.com> Message-ID: <02d471158b96dd13bd7f998f8ec2a310@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sat, 9 Sep 2017 17:47:05 +0000 X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00244.txt.bz2 On 2017-09-09 17:35, Tom Tromey wrote: > This changes print_thread_info_1 to use ui_out_emit_table and > ui_out_emit_list. Which one is used depends on whether the ui-out is > mi-like; so the emitters are wrapped in gdb::optional. LGTM. I think overall this function is a bad example of how to share code between CLI and MI. There are so many if (is_mi_like_p) that it's essentially two functions in one. Apart from iterating on threads, the MI and CLI outputs don't share much... > @@ -1247,55 +1248,55 @@ print_thread_info_1 (struct ui_out *uiout, > char *requested_threads, > update_thread_list (); > current_ptid = inferior_ptid; > > - struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); > - > - /* For backward compatibility, we make a list for MI. A table is > - preferable for the CLI, though, because it shows table > - headers. */ > - if (uiout->is_mi_like_p ()) > - make_cleanup_ui_out_list_begin_end (uiout, "threads"); > - else > - { > - int n_threads = 0; > + { > + /* For backward compatibility, we make a list for MI. A table is > + preferable for the CLI, though, because it shows table > + headers. */ > + gdb::optional list_emitter; > + gdb::optional table_emitter; > + > + if (uiout->is_mi_like_p ()) > + list_emitter.emplace (uiout, "threads"); > + else > + { > + int n_threads = 0; > > - for (tp = thread_list; tp; tp = tp->next) > - { > - if (!should_print_thread (requested_threads, default_inf_num, > - global_ids, pid, tp)) > - continue; > + for (tp = thread_list; tp; tp = tp->next) > + { > + if (!should_print_thread (requested_threads, default_inf_num, > + global_ids, pid, tp)) > + continue; > > - ++n_threads; > - } > + ++n_threads; > + } > > - if (n_threads == 0) > - { > - if (requested_threads == NULL || *requested_threads == '\0') > - uiout->message (_("No threads.\n")); > - else > - uiout->message (_("No threads match '%s'.\n"), > - requested_threads); > - do_cleanups (old_chain); > - return; > - } > + if (n_threads == 0) > + { > + if (requested_threads == NULL || *requested_threads == '\0') > + uiout->message (_("No threads.\n")); > + else > + uiout->message (_("No threads match '%s'.\n"), > + requested_threads); > + return; > + } > > - if (show_global_ids || uiout->is_mi_like_p ()) > - make_cleanup_ui_out_table_begin_end (uiout, 5, n_threads, "threads"); > - else > - make_cleanup_ui_out_table_begin_end (uiout, 4, n_threads, "threads"); > + table_emitter.emplace (uiout, > + (show_global_ids || uiout->is_mi_like_p ()) > + ? 5 : 4, > + n_threads, "threads"); > > - uiout->table_header (1, ui_left, "current", ""); > + uiout->table_header (1, ui_left, "current", ""); > > - if (!uiout->is_mi_like_p ()) > - uiout->table_header (4, ui_left, "id-in-tg", "Id"); > - if (show_global_ids || uiout->is_mi_like_p ()) > - uiout->table_header (4, ui_left, "id", "GId"); > - uiout->table_header (17, ui_left, "target-id", "Target Id"); > - uiout->table_header (1, ui_left, "frame", "Frame"); > - uiout->table_body (); > - } > + if (!uiout->is_mi_like_p ()) > + uiout->table_header (4, ui_left, "id-in-tg", "Id"); > + if (show_global_ids || uiout->is_mi_like_p ()) > + uiout->table_header (4, ui_left, "id", "GId"); > + uiout->table_header (17, ui_left, "target-id", "Target Id"); > + uiout->table_header (1, ui_left, "frame", "Frame"); > + uiout->table_body (); > + } Actually, we could remove a lot of if (is_mi_like_p) from here, since they are already in an else branch of the same check. I'll wait until this patch has been merged to clean that up, so you don't have a conflict. Thanks, Simon