Richard W.M. Jones
2020-Jan-23 15:16 UTC
Re: [Libguestfs] [common PATCH] Trim whitespaces from commands read from file
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@redhat.com> > > 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 inI 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 Rich.> > let lines = List.filter ( > > fun line -> > > String.length line > 0 && line.[0] <> '#' > >-- > >2.25.0 > > > >_______________________________________________ > >Libguestfs mailing list > >Libguestfs@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
Martin Kletzander
2020-Jan-23 21:32 UTC
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@redhat.com> >> >> 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. >I actually thought about that and it might be dangerous to just trim from left. Space in the end might change semantics of a command (although I did not go through the commands), consider something like "cp a b " which would end up being split into ["cp"; "a"; "b"; ""]. In this particular case it would probably fail, but you know what I mean, right?>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. >OK, probably the same thing needs to be added in the commit that updates the submodule in libguestfs repo, right?>ACK > >Rich. > >> > let lines = List.filter ( >> > fun line -> >> > String.length line > 0 && line.[0] <> '#' >> >-- >> >2.25.0 >> > >> >_______________________________________________ >> >Libguestfs mailing list >> >Libguestfs@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
Martin Kletzander
2020-Jan-23 22:18 UTC
Re: [Libguestfs] [common PATCH] Trim whitespaces from commands read from file
On Thu, Jan 23, 2020 at 10:32:17PM +0100, Martin Kletzander wrote:>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@redhat.com> >>> >>> 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. >> > >I actually thought about that and it might be dangerous to just trim from left. >Space in the end might change semantics of a command (although I did not go >through the commands), consider something like "cp a b " which would end up >being split into ["cp"; "a"; "b"; ""]. In this particular case it would >probably fail, but you know what I mean, right? > >>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. >> >I changed the subject to: mlcustomize: Trim whitespaces from commands read from file (RHBZ#1351000) and then realized I do not have commit access. Could anyone fix-up the subject and push it? Thanks>OK, probably the same thing needs to be added in the commit that updates the >submodule in libguestfs repo, right? >Should I send a patch so that we don't forget or is it going to be picked up anyway?>>ACK >> >>Rich. >> >>> > let lines = List.filter ( >>> > fun line -> >>> > String.length line > 0 && line.[0] <> '#' >>> >-- >>> >2.25.0 >>> > >>> >_______________________________________________ >>> >Libguestfs mailing list >>> >Libguestfs@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>_______________________________________________ >Libguestfs mailing list >Libguestfs@redhat.com >https://www.redhat.com/mailman/listinfo/libguestfs
Richard W.M. Jones
2020-Jan-24 09:29 UTC
Re: [Libguestfs] [common PATCH] Trim whitespaces from commands read from file
On Thu, Jan 23, 2020 at 10:32:17PM +0100, Martin Kletzander wrote:> On Thu, Jan 23, 2020 at 03:16:47PM +0000, Richard W.M. Jones wrote: > >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. > > > > OK, probably the same thing needs to be added in the commit that updates the > submodule in libguestfs repo, right?The script is fairly braindead, so as long as one (RHBZ#..) string appears it will work: https://github.com/libguestfs/libguestfs/blob/master/bugs-in-changelog.sh Rich. -- 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
Martin Kletzander
2020-Jan-29 12:12 UTC
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@redhat.com> >> >> 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 >> >Libguestfs@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
Martin Kletzander
2020-Jan-29 13:45 UTC
Re: [Libguestfs] [common PATCH] Trim whitespaces from commands read from file
On Wed, Jan 29, 2020 at 01:12:51PM +0100, Martin Kletzander wrote:>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@redhat.com> >>> >>> 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"]. >OK, my bad, this part of the code only splits the command from the arguments. That way it also makes more sense to use ltrim. Will send a final patch so that I get myself out of this mess =)>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 >>> >Libguestfs@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
Richard W.M. Jones
2020-Jan-29 14:23 UTC
Re: [Libguestfs] [common PATCH] Trim whitespaces from commands read from file
On Wed, Jan 29, 2020 at 01:12:51PM +0100, Martin Kletzander wrote:> 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@redhat.com> > >> > >>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?Almost everything. You can't change that library function, add a new one instead. Rich.> >Rich. > > > >>> let lines = List.filter ( > >>> fun line -> > >>> String.length line > 0 && line.[0] <> '#' > >>>-- > >>>2.25.0 > >>> > >>>_______________________________________________ > >>>Libguestfs mailing list > >>>Libguestfs@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-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Apparently Analagous Threads
- Re: [common PATCH] Trim whitespaces from commands read from file
- Re: [common PATCH] Trim whitespaces from commands read from file
- Re: [common PATCH] Trim whitespaces from commands read from file
- Re: [PATCH v2] mlcustomize: Trim whitespaces from commands read from file (RHBZ#1351000)
- [common PATCH] Trim whitespaces from commands read from file