Giter VIP home page Giter VIP logo

Comments (16)

piranout avatar piranout commented on August 14, 2024

Update: Rolling back to 1.8 from 2.1 did indeed get rid of the exception.

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

Can you please share your code for the class?

from powerargs.

piranout avatar piranout commented on August 14, 2024

Sure; I'm calling my exe with "sync" (lowercase) as the only argument. Works fine with 1.8, but produces an UnexpectedArgException ("sync") with 2.0. (Incidentally, I did not try changing case, as the command line wasn't previously case-sensitive.)

Here is the action method:

[ArgDescription("Run a sync cycle")]
[ArgActionMethod]
public static void Sync(SyncArgs syncArgs)
{
    if (!syncArgs.Quiet)
        LogProvider.Instance.Log.Debug("Sync called");
    // [...] do work...
}

And here is the SyncArgs class (the simplest form of it; there are additional properties on derived versions, but this is the one used in this case):

public class SyncArgs
{
    [ArgShortcut("q")]
    [ArgDescription("Suppress console output")]
    public bool Quiet { get; set; }
}

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

It looks like I had a regression here, but I think I found and fixed it. With 2.0 I started allowing these action methods to be instance methods instead of static, but I lost the ability to go with a static method.

If you remove the 'static' modifier then your code should work right now (please let me know if it doesn't). But I'm going to release a patch (v 2.0.2.0) that brings back the ability to have static methods for actions. I'll leave this issue open until that's out.

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

Patch released. Please let me know if your code resumes working with no change.

from powerargs.

piranout avatar piranout commented on August 14, 2024

2.0.2 worked, though unrelated changes were necessary:

  • The command parser used to tolerate action methods with the same name as a property of the arguments class (e.g. I had an action method "Help" in the command class, and a boolean property of the same name on of one of the action methods' argument classes). This caused 2.0 to throw a duplicate argument exception, so I rearranged things to accommodate (made "Help" an action only, not an argument). The only downside of this is there's no way I know of to specify "-?" as a shortcut for the action method.
  • I had an enumeration containing the names of all the action methods, and used it as a required argument in position 0 on the command class. 1.8 didn't care; it parsed the argument as an enumeration member property of the command class and invoked the action method by the same name. 2.0.2.0 didn't want to parse the enum when used that way (complained about casting arg string to an enum type), but still called the action method just fine after I got rid of the enum. Enum parsing itself is not broken mind you — it still works on action methods that I pass an enumerated value to, just not for enumerating the names of the action methods themselves in a property of the command class.

Those weren't critical design elements, so I nixed them. I think I've tested all the other scenarios, and with the changes above they work fine. So this regression is fixed, though it might be good for users to know that 2.0.x might not be a straight drop-in replacement for 1.x, depending on their command/argument structure. It would be nice to restore tolerance for action methods & action method arguments of the same name, but it's not a show-stopper. Thanks for fixing this!

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

My goal is to make 1.X -> 2.X straightforward so I'll look into restoring anything that isn't working out of the box. I'll investigate this new issue and try to resolve it (and add a test to check for future regressions).

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

Can you please elaborate on the first bullet point? I tried this code below and I'm not getting a DuplicateArgumentException.

public class ActionWithSharedPropertyNameArgs
{
    [ArgActionMethod]
    public void Duplicate(DuplicateArgsClass args)
    {
        // do stuff
    }
}

public class DuplicateArgsClass
{
    public string Duplicate { get; set; }
}

from powerargs.

piranout avatar piranout commented on August 14, 2024

Happy to... here's a bit of code (command class, enumerations, argument classes) that ought to reproduce things. I removed all the proprietary stuff that I can't post here, but that's internal method logic & shouldn't affect the command parsing behavior (hopefully). Hope this helps. [Notes: I changed the static methods to instance ones in a different branch. I prefer instance methods anyway. Also, the usages here are not necessarily the 'right' way of doing everything, just a set of hacks I cobbled together as I got familiar with PowerArgs.]

public class Command
{
    [ArgRequired]
    [ArgPosition(0)]
    [ArgDescription("Operation: Import, Extract, Show, ...")]
    public Operation Action { get; set; }

    [ArgShortcut("?")]
    [ArgShortcut("h")]
    [ArgDescription("Display the usage screen")]
    public bool Help { get; set; }

    [ArgShortcut("q")]
    [ArgDescription("Run in silent (non-interactive) mode")]
    public bool Quiet { get; set; }

    [ArgDescription("Start the service running at configured intervals")]
    [ArgActionMethod]
    public static void Start(SyncArgs syncArgs)
    {
        if (!syncArgs.Quiet)
            LogProvider.Instance.Log.Debug("Start called");
        //...create service instance
        // service.Start();
    }

    [ArgDescription("Run a single sync cycle")]
    [ArgActionMethod]
    public static void Sync(SyncArgs syncArgs)
    {
        if (!syncArgs.Quiet)
            LogProvider.Instance.Log.Debug("Sync called");
        //...create service instance
        // service.CycleOnce();
    }

    [ArgDescription("Export CSV files")]
    [ArgActionMethod]
    public static void Extract(ExtractFileArgs fileArgs)
    {
        if (Environment.UserInteractive)
            LogProvider.Instance.Log.DebugFormat("Extract beginning");
        //...create service instance
        // service.ExtractAndSave(fileArgs.ExtractTypes);
    }

    [ArgDescription("Import CSV files")]
    [ArgActionMethod]
    public static void Import(FileArgs fileArgs)
    {
        if (Environment.UserInteractive)
            LogProvider.Instance.Log.DebugFormat("Import beginning");
        //...create service instance            
        // service.Import(fileArgs);
    }
}

public enum Operation
{
    Extract,
    Import,
    Start,
    Sync
}

[Flags]
public enum ExtractType
{
    [ArgShortcut("c")]
    [ArgDescription("(c) Items that are new or have changed")]
    Changed = 4,
    [ArgShortcut("u")]
    [ArgDescription("(u) Items that have been synced before and are unchanged since")]
    Unchanged = 8,
    [ArgShortcut("n")]
    [ArgDescription("(n) Items that have never been synced before")]
    NeverSynced = 16,
    [ArgShortcut("a")]
    [ArgDescription("(a) All items, irrespective of sync history.")]
    All = Changed | Unchanged | NeverSynced
}

public class SyncArgs
{
    [ArgShortcut("q")]
    [ArgDescription("Suppress user prompts")]
    public bool Quiet { get; set; }

    [ArgShortcut("?")]
    [ArgDescription("Show help for this command instead of running")]
    public bool Help { get; set; }
}

public class FileArgs : SyncArgs
{
    [ArgExistingFile]
    public string File { get; set; }

    [ArgExistingDirectory]
    public string Directory { get; set; }

    [ArgIgnore]
    public bool IsDefault
    {
        get { return string.IsNullOrWhiteSpace(this.Directory) && string.IsNullOrWhiteSpace(this.File); }
    }
}

public class ExtractFileArgs : FileArgs
{
    private const string EXTRACT_TYPES_DESCRIPTION = @"What to extract, depending on past sync status";

    /// <summary>
    ///     What to extract - Changed (new and changed items only, default),
    ///     Unchanged (previously synced and current), Nomatch (unmatched),
    ///     All (all of the above)
    /// </summary>
    [ArgRequired]
    [ArgPosition(0)]
    [ArgDescription(EXTRACT_TYPES_DESCRIPTION)]
    public ExtractType ExtractTypes { get; set; }
}

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

I verified that this code works on 1.8, but it should have been a happy accident. The InvalidArgDefinitionException you are getting should have been thrown all along.

You don't really need the Quiet option on the child commands (especially now that I allow instance methods for Actions so you can access the parent options from within your methods. You can also do this with Help (so you don't need Help to be an action).

The other thing you did (which seems reasonable) is you used an enum for your actions. The fact that that worked in 1.8 was also an accident. I never thought of supporting it which is why its broken in 2.0. However, since I added the [ArgActionMethod] attribute you no longer even need the Action property. You can just get rid of it.

Take a look at this code. I think it's closer to how you would have wanted it to look in 1.8, but is now fully supported in 2.0. It also removes the need for the redundant SyncArgs class.

public class Command
{
    [ArgShortcut("?")]
    [ArgShortcut("h")]
    [ArgDescription("Display the usage screen")]
    public bool Help { get; set; }

    [ArgShortcut("q")]
    [ArgDescription("Run in silent (non-interactive) mode")]
    public bool Quiet { get; set; }

    [ArgDescription("Start the service running at configured intervals")]
    [ArgActionMethod]
    public void Start()
    {
        if (!Quiet)
            Console.WriteLine("Start called");
        //...create service instance
        // service.Start();
    }

    [ArgDescription("Run a single sync cycle")]
    [ArgActionMethod]
    public void Sync()
    {
        if (!Quiet)
            Console.WriteLine("Sync called");
        //...create service instance
        // service.CycleOnce();
    }

    [ArgDescription("Export CSV files")]
    [ArgActionMethod]
    public static void Extract(ExtractFileArgs fileArgs)
    {
        if (Environment.UserInteractive)
            Console.WriteLine("Extract beginning");
        //...create service instance
        // service.ExtractAndSave(fileArgs.ExtractTypes);
    }

    [ArgDescription("Import CSV files")]
    [ArgActionMethod]
    public static void Import(FileArgs fileArgs)
    {
        if (Environment.UserInteractive)
            Console.WriteLine("Import beginning");
        //...create service instance            
        // service.Import(fileArgs);
    }
}


[Flags]
public enum ExtractType
{
    [ArgShortcut("c")]
    [ArgDescription("(c) Items that are new or have changed")]
    Changed = 4,
    [ArgShortcut("u")]
    [ArgDescription("(u) Items that have been synced before and are unchanged since")]
    Unchanged = 8,
    [ArgShortcut("n")]
    [ArgDescription("(n) Items that have never been synced before")]
    NeverSynced = 16,
    [ArgShortcut("a")]
    [ArgDescription("(a) All items, irrespective of sync history.")]
    All = Changed | Unchanged | NeverSynced
}

public class FileArgs
{
    [ArgExistingFile]
    public string File { get; set; }

    [ArgExistingDirectory]
    public string Directory { get; set; }

    [ArgIgnore]
    public bool IsDefault
    {
        get { return string.IsNullOrWhiteSpace(this.Directory) && string.IsNullOrWhiteSpace(this.File); }
    }
}

public class ExtractFileArgs : FileArgs
{
    private const string EXTRACT_TYPES_DESCRIPTION = @"What to extract, depending on past sync status";

    /// <summary>
    ///     What to extract - Changed (new and changed items only, default),
    ///     Unchanged (previously synced and current), Nomatch (unmatched),
    ///     All (all of the above)
    /// </summary>
    [ArgRequired]
    [ArgPosition(0)]
    [ArgDescription(EXTRACT_TYPES_DESCRIPTION)]
    public ExtractType ExtractTypes { get; set; }
}

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

Any more thoughts on my last comment? If not, I will close this issue soon.

Thanks,
Adam

from powerargs.

piranout avatar piranout commented on August 14, 2024

That functionality is fine for me. One question though: What attribute targets are valid for an ArgShortcut attribute?

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

It's Property, Field, and Method. However, it doesn't really work on action methods. It was a mistake to open that target up, but I haven't fixed it yet.

from powerargs.

piranout avatar piranout commented on August 14, 2024

Aha, that's what I feared. I don't envy the maintenance headache it would be to make it really work on action methods. However, by maintaining the best and most robust argument parser / invoker on NuGet, you have opened yourself up to some hefty responsibilities. [Sorry to employ both flattery and duty there, but I'd really like to put ad-hoc shortcuts on my action methods... muahaha... ;-) ]

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

Ah, flattery. My weakness. I'll have to dig in sometime and make it work.

from powerargs.

adamabdelhamed avatar adamabdelhamed commented on August 14, 2024

Fixed in 2.5.0.0

from powerargs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.