[vlc-devel] [PATCH 2/2] web intf: Fix setting volume in percent

[Filip Roséen]

Hi Marco,

On 2017-04-28 18:39, vlc-devel wrote:

> From: Marco de Abreu <marco.g.abreu at googlemail.com>
> To: vlc-devel at videolan.org
> 
> Setting the volume as percentage was described in the documentation,
> but not supported in the code and raised an exception.
> It can now be set like "50%" (or "50%25" html encoded).

The above commit-message might warrant a slight rewording, as the
affected path does not at all deal with URI-encoded entities.
 
> Ticket: #16777
> ---
>  share/lua/modules/common.lua | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/share/lua/modules/common.lua b/share/lua/modules/common.lua
> index e3d3901..10bfaf2 100644
> --- a/share/lua/modules/common.lua
> +++ b/share/lua/modules/common.lua
> @@ -171,6 +171,9 @@ end
>  function volume(value)

As the following `if` + `elseif` both includes `type(value)=="string"`
it might be cleaner to write it as a nested statement, as the below
(of course untested, but you get the idea):

    if type(value) == "string" then
      if value:match("^[+-]")     then vlc.volume.set(...)
      elseif value.sub(-1) == "%" then vlc.volume.set(...)
      end
    else
      vlc.volume.set(value)
    end

>      if type(value)=="string" and string.sub(value,1,1) == "+" or string.sub(value,1,1) == "-" then
>          vlc.volume.set(vlc.volume.get()+tonumber(value))
> +    elseif type(value) == "string" and string.sub(value,-1) == "%" then
> +        -- value of 256 respresents 100% volume

The comment above includes a typo, *"represents"* is the proper
spelling.

> +        vlc.volume.set(256 * tonumber(string.sub(value,1,-2)) / 100)

I am aware of the fact that you are not introducing a bug (if one even
would call it such), but it is rather unfortunate that the usage of
`tonumber` does not include an explicit base.

It is debatable whether `volume("0xFF")` should be accepted as input,
or not.

>      else
>          vlc.volume.set(tostring(value))

The above usage of `tostring` is strictly speaking superfluous.

>      end
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170510/35a182b8/attachment.html>