<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
  <meta http-equiv="Content-Style-Type" content="text/css" />
  <meta name="generator" content="pandoc" />
  <title></title>
  <style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Marco,</p>
<p>On 2017-04-28 18:39, vlc-devel wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> From: Marco de Abreu <marco.g.abreu@googlemail.com>
 To: vlc-devel@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).</code></pre>
</blockquote>
<p>The above commit-message might warrant a slight rewording, as the affected path does not at all deal with URI-encoded entities.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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)</code></pre>
</blockquote>
<p>As the following <code>if</code> + <code>elseif</code> both includes <code>type(value)=="string"</code> it might be cleaner to write it as a nested statement, as the below (of course untested, but you get the idea):</p>
<pre><code>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</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>      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</code></pre>
</blockquote>
<p>The comment above includes a typo, <em>“represents”</em> is the proper spelling.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> +        vlc.volume.set(256 * tonumber(string.sub(value,1,-2)) / 100)</code></pre>
</blockquote>
<p>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 <code>tonumber</code> does not include an explicit base.</p>
<p>It is debatable whether <code>volume("0xFF")</code> should be accepted as input, or not.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>      else
          vlc.volume.set(tostring(value))</code></pre>
</blockquote>
<p>The above usage of <code>tostring</code> is strictly speaking superfluous.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>      end</code></pre>
</blockquote>
</body>
</html>