GNOME Bugzilla – Bug 620684
switch commandline parsing to hyena
Last modified: 2010-06-08 17:08:04 UTC
I want to port F-spots command line handling to Hyena. While working on this I found out that Hyena.CommandLine only supports options that are marked with "--". F-spot currently has many "-" options, so this would mean some kind of break of functionality. I think this is acceptable but would like to hear more opinions.
Created attachment 162888 [details] [review] [1/1] Port command line parsing to Hyena.CommandLine src/main.cs | 272 +++++++++++++++++++++++++++++++++-------------------------- 1 files changed, 151 insertions(+), 121 deletions(-)
Two questions: should we update copyright and add a license header?
Don't bother with copyrights and license stuff, I am relicensing F-Spot and will then do it all in one go. Will look into your patch very soon, but from a first glance it looks good.
Hi Paul, I'm looking at your patch but it seems to be a plain git diff and I'm getting some conflicts. Could you: * git commit all your changes * then do: git pull --rebase * (if needed fix any conflicts, git add the conflicting files and git rebase --continue) * git format-patch And attach the resulting patch? That way it'll work and the patch will have a good commit message + authorship information.
Created attachment 162965 [details] [review] update patch now using git format-patch.
Review of attachment 162965 [details] [review]: Clean patch (with two comments, see below). We should also update the launcher script (src/f-spot.in) to provide backwards compatibility with the short options (-h, -?, -b, -p, -i, -v, -V). No new short options will be accepted in the future. Once those are fixed, it can go in for me. ::: src/main.cs @@ -81,5 +108,5 @@ - for (int i = 0; i < args.Length && !shutdown; i++) { - switch (args [i]) { - case "-h": case "-?": case "-help": case "--help": case "-usage": ... 2 more ... + + if (ApplicationContext.CommandLine.ContainsStart ("help")) { + ShowHelp (); ... 2 more ... F-Spot should probably exit after showing the help message. @@ +112,3 @@ + + if (ApplicationContext.CommandLine.Contains ("version")) { + ShowVersion (); Same thing with version information.
Created attachment 163043 [details] [review] Port commandline options parsing to Hyena.CommandLine library.
updated patch. I didn't find anything in f-spot.in to update. thanks for review.
Please mark the old version as obsolete when submitting the new patch.
Created attachment 163076 [details] [review] Port commandline options parsing to Hyena.CommandLine library.
Attachment 163076 [details] pushed as cd79bfe - Port commandline options parsing to Hyena.CommandLine library. Fixed, with small additions. Thanks a lot! This turned out to be more complex than I expected, nice work!