Re: [Libguestfs] [common PATCH] Trim whitespaces from commands read from file

On Thu, Jan 23, 2020 at 03:16:47PM +0000, Richard W.M. Jones wrote: >On Thu, Jan 23, 2020 at 02:04:53PM +0100, Martin Kletzander wrote: >>On Thu, Jan 23, 2020 at 12:16:50PM +0100, Martin Kletzander wrote: >>>Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; >> >>I forgot to mention this is trying to fix the following BZ: >> >>https://bugzilla.redhat.com/show_bug.cgi?id=1351000 > >>>--- >>>mlcustomize/customize_cmdline.ml | 1 + >>>1 file changed, 1 insertion(+) >>> >>>diff --git a/mlcustomize/customize_cmdline.ml b/mlcustomize/customize_cmdline.ml >>>index c062379879e2..67e85af2ad93 100644 >>>--- a/mlcustomize/customize_cmdline.ml >>>+++ b/mlcustomize/customize_cmdline.ml >>>@@ -481,6 +481,7 @@ let rec argspec () = >>> ] in >>> let lines = read_whole_file filename in >>> let lines = String.lines_split lines in >>>+ let lines = List.map String.trim lines in > >I wonder if String.triml is safer? However I cannot >think of a way that it's likely to be useful to have >trailing whitespace be meaningful. > >Please put this in the subject line of the commit >message: “(RHBZ#1351000)”. This will ensure that it >is picked up by the release notes by the >bugs-in-changelog script. > >ACK > I found that this is still not enough to make it behave in a nice way. What we probably want is the split, which happens few lines lower, to actually behave similar to Str.split, which actually splits by any number of the delimiters (so that "asdf fdsa" is split into ["asdf"; "fdsa"] and not ["asdf"; ""; "fdsa"]. I know this fix is not important at all, but since I wanted to exercise some OCaml, I will try to fix that implementation instead. Do any one of you know of a part of any libguestfs tool which would depend on the current, too strict, behaviour?

>Rich. > >>> let lines = List.filter ( >>> fun line -> >>> String.length line > 0 && line.[0] <> '#' >>>-- >>>2.25.0 >>> >>>_______________________________________________ >>>Libguestfs mailing list &gt;&gt;&gt;Libguestfs(a)redhat.com >>>https://www.redhat.com/mailman/listinfo/libguestfs > > > >-- >Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones >Read my programming and virtualization blog: http://rwmj.wordpress.com >virt-top is 'top' for virtual machines. Tiny program with many >powerful monitoring features, net stats, disk stats, logging, etc. >http://people.redhat.com/~rjones/virt-top