duncs / clusterssh

Cluster SSH - Cluster Admin Via SSH
https://github.com/duncs/clusterssh/wiki
896 stars 79 forks source link

debug level argument doesn't work #85

Closed fastcat closed 7 years ago

fastcat commented 7 years ago

The command line arg to set debug levels ... largely doesn't work. Because the code that tries to set it, only tries to set it on the Getopt instance ... and even that it does totally wrong.

The code in Getopt.pm needs to look something more like this:

    $self->set_debug_level( $options->{debug} );
    $self->parent->set_debug_level( $options->{debug} );

But even that isn't enough, because any time an object deriving from ClusterSSH::Base is created that doesn't explicitly set a debug level in its constructor arguments, the global debug level is slammed back to zero again, because of this code:

    my $config = {
        lang  => 'en',
        debug => 0,
        %args,
    };
# ...
    $self->set_debug_level( $config->{debug} );

Something like this would be marginally better, but still has the oddity of setting a global debug level value when you construct an instance of something:

    $self->set_debug_level( $config->{debug} ) if defined $args{debug};

Side note: The "noexit" debug config dump ought to be moved to after the wm decoration detection so that the results of that are visible in that dump. I.e. move it to just before # now we have the info, plot the first window position

duncs commented 7 years ago

I have already made some changes to the repo code as I found problems with debug recently. Are you looking at the repo or in a released version?

Duncs

On 26 Apr 2017, at 21:02, Matthew Gabeler-Lee notifications@github.com wrote:

The command line arg to set debug levels ... largely doesn't work. Because the code that tries to set it, only tries to set it on the Getopt instance ... and even that it does totally wrong.

The code in Getopt.pm needs to look something more like this:

$self->set_debug_level( $options->{debug} );
$self->parent->set_debug_level( $options->{debug} );

But even that isn't enough, because any time an object deriving from ClusterSSH::Base is created that doesn't explicitly set a debug level in its constructor arguments, the global debug level is slammed back to zero again, because of this code:

my $config = {
    lang  => 'en',
    debug => 0,
    %args,
};

...

$self->set_debug_level( $config->{debug} );

Something like this would be marginally better, but still has the oddity of setting a global debug level value when you construct an instance of something:

$self->set_debug_level( $config->{debug} ) if defined $args{debug};

Side note: The "noexit" debug config dump ought to be moved to after the wm decoration detection so that the results of that are visible in that dump. I.e. move it to just before # now we have the info, plot the first window position

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

fastcat commented 7 years ago

I was looking at the released version.

duncs commented 7 years ago

I have released a new version (4.10_2) - please try it out and if you have any further problems let me know.