<p><a href="https://github.com/codebrainz" class="user-mention">@codebrainz</a></p>
<p>There are plenty of tools out there that can remove NULs if that is actually the correct thing to do to the file (which its probably not if its an incorrect encoding).  So its not a good trade-off of effort vs return to add the capability to Geany for an occasional faulty file.  Just because its possible to do, doesn't mean it should be done.</p>
<blockquote>
<p>Have you actually checked how many functions used in Geany would be affected? I haven't, but I ask because it might be worth actually checking to see, maybe it's not so many more as one might assume.</p>
</blockquote>
<p>I don't support adding editing so I won't do that. :)</p>
<p><a href="https://github.com/b4n" class="user-mention">@b4n</a></p>
<blockquote>
<p>some encodings might generate NUL bytes in their encoded stream anyway (or at least could anyway)</p>
</blockquote>
<p>Sure, for example an ASCII file saved in UTF-16 will be full of NUL bytes, and it would be a bug if we did not allow for that, but thats different to NUL in the UTF-8.  (and saving a file with "foo" as UTF-16 indeed works fine despite every second byte being NUL :)</p>
<blockquote>
<p>Anyway: what I fixed was simply that now we properly write the whole thing in any case, with no corner case doing otherwise.</p>
</blockquote>
<p>Ok, a silent truncation on saving of a buffer with a NUL is a "bad thing" <g-emoji alias="tm" fallback-src="https://assets-cdn.github.com/images/icons/emoji/unicode/2122.png" ios-version="6.0">™️</g-emoji> , for example if a buggy plugin made a NUL.</p>
<blockquote>
<p>Non-regex search doesn't, however (could be interesting to fix, but for now it's out of the scope of this).</p>
</blockquote>
<p>So long as there have been comprehensive tests that <strong>nothing</strong> that accesses the readonly buffer with NULs can (1) crash (2) hang (3) cause problems for other buffers (and that includes testing all plugins).</p>
<p>Note I didn't say anything had to work right, just not cause problems.</p>
<blockquote>
<p>So <a href="https://github.com/elextr" class="user-mention">@elextr</a> I think the goal of this PR matches your hopes: loading files, but nothing else -- and warning users it's not properly supported and can cause issues.</p>
</blockquote>
<p>And to be clear I support that if there is a proposal for how to do the comprehensive testing I mentioned above, and will review this when I get a chance.</p>
<blockquote>
<p>But if one day we end up with full support for those, I don't think it would be a bad thing.</p>
</blockquote>
<p>And as I said above, just because we <strong>can</strong> doesn't mean we <strong>should</strong> waste time on it when there are many other things to be done.  And don't forget plugins.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/geany/geany/pull/1709#issuecomment-349472427">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABDrJ-0a5A7BvXh8qwqm2JUXPOtaD35Yks5s9c1BgaJpZM4Q18TU">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABDrJ-r1TfPYxLQBD_TtDUXtlc5bvhohks5s9c1BgaJpZM4Q18TU.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/geany/geany/pull/1709#issuecomment-349472427"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/geany/geany","title":"geany/geany","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/geany/geany"}},"updates":{"snippets":[{"icon":"PERSON","message":"@elextr in #1709: @codebrainz \r\n\r\nThere are plenty of tools out there that can remove NULs if that is actually the correct thing to do to the file (which its probably not if its an incorrect encoding).  So its not a good trade-off of effort vs return to add the capability to Geany for an occasional faulty file.  Just because its possible to do, doesn't mean it should be done.\r\n\r\n\u003e Have you actually checked how many functions used in Geany would be affected? I haven't, but I ask because it might be worth actually checking to see, maybe it's not so many more as one might assume.\r\n\r\nI don't support adding editing so I won't do that. :)\r\n\r\n@b4n \r\n\r\n\u003e some encodings might generate NUL bytes in their encoded stream anyway (or at least could anyway)\r\n\r\nSure, for example an ASCII file saved in UTF-16 will be full of NUL bytes, and it would be a bug if we did not allow for that, but thats different to NUL in the UTF-8.  (and saving a file with \"foo\" as UTF-16 indeed works fine despite every second byte being NUL :)\r\n\r\n\u003e Anyway: what I fixed was simply that now we properly write the whole thing in any case, with no corner case doing otherwise.\r\n\r\nOk, a silent truncation on saving of a buffer with a NUL is a \"bad thing\" :tm: , for example if a buggy plugin made a NUL.\r\n\r\n\u003e Non-regex search doesn't, however (could be interesting to fix, but for now it's out of the scope of this).\r\n\r\nSo long as there have been comprehensive tests that __nothing__ that accesses the readonly buffer with NULs can (1) crash (2) hang (3) cause problems for other buffers (and that includes testing all plugins).\r\n\r\nNote I didn't say anything had to work right, just not cause problems.  \r\n\r\n\u003e So @elextr I think the goal of this PR matches your hopes: loading files, but nothing else -- and warning users it's not properly supported and can cause issues.\r\n\r\nAnd to be clear I support that if there is a proposal for how to do the comprehensive testing I mentioned above, and will review this when I get a chance.\r\n\r\n\u003e But if one day we end up with full support for those, I don't think it would be a bad thing.\r\n\r\nAnd as I said above, just because we __can__ doesn't mean we __should__ waste time on it when there are many other things to be done.  And don't forget plugins."}],"action":{"name":"View Pull Request","url":"https://github.com/geany/geany/pull/1709#issuecomment-349472427"}}}</script>