Return-Path: <linux-bluetooth-owner@vger.kernel.org>
MIME-Version: 1.0
In-Reply-To: <CABBYNZJ05njFZbt7JQ6_P0Cz-sEK2HbUN0PEsRU8yYUEmx-jdg@mail.gmail.com>
References: <20170918092325.32406-1-luiz.dentz@gmail.com> <1a507085-2113-dd3b-3bf3-07c55d359a9b@jp.fujitsu.com>
 <CABBYNZJ05njFZbt7JQ6_P0Cz-sEK2HbUN0PEsRU8yYUEmx-jdg@mail.gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Wed, 20 Sep 2017 15:49:42 +0300
Message-ID: <CABBYNZ+H5ruWjn-Fddi-iNDR1xucdBddnNYi60zOG1XhfixGag@mail.gmail.com>
Subject: Re: [PATCH BlueZ] tools: Only add unique entries to readline history
To: ERAMOTO Masaya <eramoto.masaya@jp.fujitsu.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Hi,

On Tue, Sep 19, 2017 at 10:08 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Eramoto,
>
> On Tue, Sep 19, 2017 at 5:14 AM, ERAMOTO Masaya
> <eramoto.masaya@jp.fujitsu.com> wrote:
>> Hi Luiz,
>>
>> On 09/18/2017 06:23 PM, Luiz Augusto von Dentz wrote:
>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>
>>> Don't add duplicate commands to historym this is similar to what
>>> HISTCONTROL=ignoredups does.
>>> ---
>>>  client/main.c            | 3 ++-
>>>  tools/bluetooth-player.c | 4 +++-
>>>  tools/btmgmt.c           | 3 ++-
>>>  tools/obex-client-tool.c | 3 ++-
>>>  tools/obexctl.c          | 3 ++-
>>>  5 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/client/main.c b/client/main.c
>>> index 2cb449fd5..282e84003 100644
>>> --- a/client/main.c
>>> +++ b/client/main.c
>>> @@ -2644,7 +2644,8 @@ static void rl_handler(char *input)
>>>       if (!rl_release_prompt(input))
>>>               goto done;
>>>
>>> -     add_history(input);
>>> +     if (history_search(input, -1))
>>> +             add_history(input);
>>>
>>
>> I think it is better to add g_strstrip(), since this patch adds a
>> command typed all characters to history and then adds the complemented
>> command to history when it have white-space.
>
> Yep, but as a separate change, this part of handling should really go
> into a common code so we don't have to keep duplicating this on every
> tool.
>
>>>       cmd = strtok_r(input, " ", &arg);
>>>       if (!cmd)
>>> diff --git a/tools/bluetooth-player.c b/tools/bluetooth-player.c
>>> index 9e199970d..c95b7497f 100644
>>> --- a/tools/bluetooth-player.c
>>> +++ b/tools/bluetooth-player.c
>>> @@ -1086,7 +1086,9 @@ static void rl_handler(char *input)
>>>               goto done;
>>>
>>>       g_strstrip(input);
>>> -     add_history(input);
>>> +
>>> +     if (history_search(input, -1))
>>> +             add_history(input);
>>>
>>>       argv = g_strsplit(input, " ", -1);
>>>       if (argv == NULL)
>>> diff --git a/tools/btmgmt.c b/tools/btmgmt.c
>>> index e7ea69937..e454d864d 100644
>>> --- a/tools/btmgmt.c
>>> +++ b/tools/btmgmt.c
>>> @@ -4664,7 +4664,8 @@ static void rl_handler(char *input)
>>>       if (prompt_input(input))
>>>               goto done;
>>>
>>> -     add_history(input);
>>> +     if (history_search(input, -1))
>>> +             add_history(input);
>>>
>>>       if (wordexp(input, &w, WRDE_NOCMD))
>>>               goto done;
>>> diff --git a/tools/obex-client-tool.c b/tools/obex-client-tool.c
>>> index d0ba8a651..d74ac77a1 100644
>>> --- a/tools/obex-client-tool.c
>>> +++ b/tools/obex-client-tool.c
>>> @@ -242,7 +242,8 @@ static void parse_line(char *line_read)
>>>               return;
>>>       }
>>>
>>> -     add_history(line_read);
>>> +     if (history_search(input, -1))
>>> +             add_history(input);
>>
>> line_read is correct.
>
> Good catch, will fix it.
>
>>>
>>>       g_shell_parse_argv(line_read, &argcp, &argvp, NULL);
>>>
>>> diff --git a/tools/obexctl.c b/tools/obexctl.c
>>> index 46943d682..ece50f682 100644
>>> --- a/tools/obexctl.c
>>> +++ b/tools/obexctl.c
>>> @@ -2081,7 +2081,8 @@ static void rl_handler(char *input)
>>>       if (!strlen(input))
>>>               goto done;
>>>
>>> -     add_history(input);
>>> +     if (history_search(input, -1))
>>> +             add_history(input);
>>>
>>>       if (wordexp(input, &w, WRDE_NOCMD))
>>>               goto done;
>>>
>>
>>
>> Regards,
>> Eramoto
>>

Applied.

-- 
Luiz Augusto von Dentz