Skip to content

Conversation

@Grantim
Copy link
Contributor

@Grantim Grantim commented Oct 10, 2025

No description provided.

@Grantim Grantim requested review from Fedr and oitel October 10, 2025 10:14
Comment on lines 38 to 42
cfg.reset( appName );
if ( reset )
{
cfg.setFullConfig( Json::Value() );
cfg.reset( appName );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cfg.reset( appName );
if ( reset )
{
cfg.setFullConfig( Json::Value() );
cfg.reset( appName );
}
if ( reset )
cfg.setFullConfig( Json::Value() );
cfg.reset( appName );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will not work this way, let me add comment

/// Sets custom viewer settings manager to viewer
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName ) const;
/// reset - flags indicates that user launched application with request for reseting config file
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName, bool reset ) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName, bool reset ) const;
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName, bool reset = false ) const;

MRMESH_API void setJsonValue( const std::string& key, const Json::Value& keyValue );

/// returns json with content of this config
Json::Value getFullConfig() const { return config_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toJson?

Json::Value getFullConfig() const { return config_; }

/// replace current config content with given one
void setFullConfig( const Json::Value& config ) { config_ = config; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setFromJson?

Comment on lines 38 to 42
cfg.reset( appName );
if ( reset )
{
cfg.setFullConfig( Json::Value() );
cfg.reset( appName );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to call setFullConfig between two resets rather than before one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately

/// Sets custom viewer settings manager to viewer
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName ) const;
/// reset - flags indicates that user launched application with request for reseting config file
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName, bool reset ) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool reset = false by default?


/// Sets custom viewer settings manager to viewer
MRVIEWER_API virtual void setupSettingsManager( Viewer* viewer, const std::string& appName ) const;
/// reset - flags indicates that user launched application with request for reseting config file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\param reset

@Grantim Grantim merged commit 8d4577c into master Oct 10, 2025
35 checks passed
@Grantim Grantim deleted the Add_option_to_force_reset_config_on_start_of_application branch October 10, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants