Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

616 double click image #617

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions meson_options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,10 @@ option(
value: 'false',
description: 'enable -fsanitize=address compile/link flag',
)

option(
'config-debug',
type: 'feature',
value: 'auto',
description: 'enable CONFIG_DEBUG, some unittests need this flag',
Copy link

Choose a reason for hiding this comment

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

suggestion (documentation): Change 'unittests' to 'unit tests' for clarity.

Consider changing 'unittests' to 'unit tests' to improve readability and adhere to common terminology.

Suggested change
description: 'enable CONFIG_DEBUG, some unittests need this flag',
description: 'enable CONFIG_DEBUG, some unit tests need this flag',

)
1 change: 1 addition & 0 deletions src/TestConfig.h.in
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef IPTUX_TEST_CONFIG_H
#define IPTUX_TEST_CONFIG_H

#include "config.h"
#mesondefine CURRENT_SOURCE_PATH

#endif // IPTUX_TEST_CONFIG_H
1 change: 1 addition & 0 deletions src/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define PHOTO_PATH "/iptux/photo"

#mesondefine SYSTEM_DARWIN
#mesondefine CONFIG_DEBUG

#if defined(__APPLE__) || defined(__CYGWIN__)
#define O_LARGEFILE 0
Expand Down
3 changes: 3 additions & 0 deletions src/iptux/ApplicationTest.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "TestConfig.h"
#include "UiHelper.h"
#include "gtest/gtest.h"

Expand All @@ -13,6 +14,7 @@ void do_action(Application* app, const string& name) {
g_action_activate(g_action_map_lookup_action(m, name.c_str()), NULL);
}

#if CONFIG_DEBUG
TEST(Application, Constructor) {
_ForTestToggleOpenUrl(false);
Application* app = CreateApplication();
Expand All @@ -26,3 +28,4 @@ TEST(Application, Constructor) {
app->_ForTestProcessEvents();
DestroyApplication(app);
}
#endif
3 changes: 3 additions & 0 deletions src/iptux/DataSettingsTest.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "TestConfig.h"
#include "UiHelper.h"
#include "gtest/gtest.h"

Expand All @@ -15,6 +16,7 @@ TEST(DataSettings, Constructor) {
DestroyApplication(app);
}

#if CONFIG_DEBUG
TEST(DataSettings, Constructor2) {
pop_disable();
Application* app = CreateApplication();
Expand All @@ -23,3 +25,4 @@ TEST(DataSettings, Constructor2) {
gtk_entry_set_text(port_entry, "abc");
ASSERT_FALSE(ds.Save());
}
#endif
31 changes: 24 additions & 7 deletions src/iptux/DialogBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,25 @@
gtk_text_buffer_delete_mark(buffer, mark);
}

typedef struct _GetImageCbkCtx {
int idx;
GtkEventBox* res;
} GetImageCbkCtx;

static void GetImageCbk(GtkWidget* widget, GetImageCbkCtx* ctx) {
if (GTK_IS_EVENT_BOX(widget) && ctx->idx-- == 0) {
ctx->res = GTK_EVENT_BOX(widget);
}
}

GtkEventBox* DialogBase::chatHistoryGetImageEventBox(int idx) {
Copy link

Choose a reason for hiding this comment

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

issue: Check for negative index values.

Consider adding a check to ensure that the idx parameter is non-negative. Negative values could lead to unexpected behavior.

GetImageCbkCtx ctx = {idx, nullptr};

gtk_container_foreach(GTK_CONTAINER(chat_history_widget),
(GtkCallback)GetImageCbk, &ctx);
return ctx.res;
}

/**
* 窗口打开情况下,新消息来到以后的接口
*/
Expand Down Expand Up @@ -818,14 +837,12 @@
m_imagePopupMenu = GTK_MENU(gtk_menu_new());

GtkWidget* menu_item = gtk_menu_item_new_with_label(_("Save Image"));
gtk_actionable_set_action_name(GTK_ACTIONABLE(menu_item), "win.save_image");
gtk_menu_shell_append(GTK_MENU_SHELL(m_imagePopupMenu), menu_item);
g_signal_connect_swapped(menu_item, "activate",
G_CALLBACK(DialogBase::OnSaveImage), this);

menu_item = gtk_menu_item_new_with_label(_("Copy Image"));
gtk_actionable_set_action_name(GTK_ACTIONABLE(menu_item), "win.copy_image");
gtk_menu_shell_append(GTK_MENU_SHELL(m_imagePopupMenu), menu_item);
g_signal_connect_swapped(menu_item, "activate",
G_CALLBACK(DialogBase::OnCopyImage), this);

gtk_menu_attach_to_widget(m_imagePopupMenu, GTK_WIDGET(getWindow()), NULL);
}
Expand Down Expand Up @@ -880,7 +897,7 @@
gtk_widget_show_all(event_box);
}

void DialogBase::OnSaveImage(DialogBase* self) {
void DialogBase::onSaveImage(void*, void*, DialogBase* self) {

Check warning on line 900 in src/iptux/DialogBase.cpp

View check run for this annotation

Codecov / codecov/patch

src/iptux/DialogBase.cpp#L900

Added line #L900 was not covered by tests
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider renaming parameters for clarity.

The parameters 'void*, void*' in the onSaveImage and onCopyImage functions are not descriptive. Consider renaming them to provide more context about their purpose.

GtkImage* image = self->m_activeImage;
g_return_if_fail(!!image);

Expand All @@ -899,7 +916,7 @@
TRUE);
gtk_file_chooser_set_current_name(GTK_FILE_CHOOSER(dialog), "image.png");

if (gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
if (igtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {

Check warning on line 919 in src/iptux/DialogBase.cpp

View check run for this annotation

Codecov / codecov/patch

src/iptux/DialogBase.cpp#L919

Added line #L919 was not covered by tests
char* save_path = gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));

GError* error = NULL;
Expand All @@ -920,7 +937,7 @@
gtk_widget_destroy(dialog);
}

void DialogBase::OnCopyImage(DialogBase* self) {
void DialogBase::onCopyImage(void*, void*, DialogBase* self) {

Check warning on line 940 in src/iptux/DialogBase.cpp

View check run for this annotation

Codecov / codecov/patch

src/iptux/DialogBase.cpp#L940

Added line #L940 was not covered by tests
GtkImage* image = self->m_activeImage;
g_return_if_fail(!!image);

Expand Down
5 changes: 3 additions & 2 deletions src/iptux/DialogBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class DialogBase : public SessionAbstract, public sigc::trackable {

virtual GtkWindow* getWindow() = 0;
void ClearHistoryTextView();
GtkEventBox* chatHistoryGetImageEventBox(int idx);

protected:
void InitSublayerGeneral();
Expand Down Expand Up @@ -91,8 +92,8 @@ class DialogBase : public SessionAbstract, public sigc::trackable {
const GtkTextIter* location,
GtkTextChildAnchor* anchor,
GtkTextBuffer* buffer);
static void OnSaveImage(DialogBase* self);
static void OnCopyImage(DialogBase* self);
static void onSaveImage(void*, void*, DialogBase* self);
static void onCopyImage(void*, void*, DialogBase* self);

protected:
Application* app;
Expand Down
2 changes: 2 additions & 0 deletions src/iptux/DialogPeer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ void DialogPeer::init() {
G_ACTION_CALLBACK(onRequestSharedResources)),
makeActionEntry("send_message", G_ACTION_CALLBACK(onSendMessage)),
makeActionEntry("paste", G_ACTION_CALLBACK(onPaste)),
makeActionEntry("save_image", G_ACTION_CALLBACK(onSaveImage)),
makeActionEntry("copy_image", G_ACTION_CALLBACK(onCopyImage)),
};
g_action_map_add_action_entries(G_ACTION_MAP(window), win_entries,
G_N_ELEMENTS(win_entries), this);
Expand Down
6 changes: 6 additions & 0 deletions src/iptux/DialogPeerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,11 @@ TEST(DialogPeer, Constructor) {
ChipData(MessageContentType::PICTURE, testDataPath("iptux.png")));
grpinf->addMsgPara(msg);

GtkEventBox* eb = dlgpr->chatHistoryGetImageEventBox(0);
ASSERT_NE(eb, nullptr);
GtkImage* image = GTK_IMAGE(gtk_bin_get_child(GTK_BIN(eb)));
ASSERT_NE(image, nullptr);
gtk_widget_grab_focus(GTK_WIDGET(eb));

DestroyApplication(app);
}
39 changes: 30 additions & 9 deletions src/iptux/UiHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,17 @@

namespace iptux {

static atomic_bool open_url_enabled(true);
#if CONFIG_DEBUG
static bool pop_disabled = false;
static atomic_bool open_url_enabled(true);
static gint igtk_dialog_run_return_val = 0;
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Consider thread safety for igtk_dialog_run_return_val.

Since igtk_dialog_run_return_val is a global variable, it might be accessed by multiple threads simultaneously. Consider using an atomic type or adding synchronization mechanisms to ensure thread safety.

#else
enum {
igtk_dialog_run_return_val = 0,
pop_disabled = 0,
open_url_enabled = 1,
};
Comment on lines +24 to +28
Copy link

Choose a reason for hiding this comment

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

suggestion: Avoid using enum for non-enumeration constants.

Using an enum to define constants like igtk_dialog_run_return_val, pop_disabled, and open_url_enabled can be misleading since enums are typically used for related sets of constants. Consider using const or constexpr instead.

Suggested change
enum {
igtk_dialog_run_return_val = 0,
pop_disabled = 0,
open_url_enabled = 1,
};
static const gint igtk_dialog_run_return_val = 0;
static const gint pop_disabled = 0;
static const gint open_url_enabled = 1;

#endif

void iptux_open_path(const char* path) {
g_return_if_fail(!!path);
Expand All @@ -42,10 +51,6 @@
g_free(uri);
}

void _ForTestToggleOpenUrl(bool enable) {
open_url_enabled = enable;
}

/**
* 打开URL.
* @param url url
Expand Down Expand Up @@ -156,10 +161,6 @@
return filelist;
}

void pop_disable() {
pop_disabled = true;
}

/**
* 弹出消息提示.
* @param parent parent window
Expand Down Expand Up @@ -399,4 +400,24 @@
return res;
}

gint igtk_dialog_run(GtkDialog* dialog) {
if (igtk_dialog_run_return_val) {
return igtk_dialog_run_return_val;

Check warning on line 405 in src/iptux/UiHelper.cpp

View check run for this annotation

Codecov / codecov/patch

src/iptux/UiHelper.cpp#L403-L405

Added lines #L403 - L405 were not covered by tests
}
return gtk_dialog_run(dialog);

Check warning on line 407 in src/iptux/UiHelper.cpp

View check run for this annotation

Codecov / codecov/patch

src/iptux/UiHelper.cpp#L407

Added line #L407 was not covered by tests
}

#if CONFIG_DEBUG
void pop_disable() {
pop_disabled = true;
}
void _ForTestToggleOpenUrl(bool enable) {
open_url_enabled = enable;
}

void setIgtkDialogRunReturnVal(gint val) {
igtk_dialog_run_return_val = val;
}

Check warning on line 420 in src/iptux/UiHelper.cpp

View check run for this annotation

Codecov / codecov/patch

src/iptux/UiHelper.cpp#L418-L420

Added lines #L418 - L420 were not covered by tests
#endif

} // namespace iptux
17 changes: 11 additions & 6 deletions src/iptux/UiHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,12 @@ GSList* selection_data_get_path(GtkSelectionData* data);
*/
GtkImage* igtk_image_new_with_size(const char* filename, int width, int height);
std::string igtk_text_buffer_get_text(GtkTextBuffer* buffer);
gint igtk_dialog_run(GtkDialog* dialog);

/**
* @brief only used for test, after call this, pop_info, pop_warning,
* and iptux_open_url will only print log
*/
void pop_disable();
void pop_info(GtkWidget* parent, const gchar* format, ...) G_GNUC_PRINTF(2, 3);
void pop_warning(GtkWidget* parent, const gchar* format, ...)
G_GNUC_PRINTF(2, 3);
void iptux_open_url(const char* url);
void _ForTestToggleOpenUrl(bool enable);

std::string ipv4_get_lan_name(in_addr ipv4);

Expand Down Expand Up @@ -107,5 +102,15 @@ std::string TimeToStr(time_t t);
/* only used for test */
std::string TimeToStr_(time_t t, time_t now);

#if CONFIG_DEBUG
/**
* @brief only used for test, after call this, pop_info, pop_warning,
* and iptux_open_url will only print log
*/
void pop_disable();
void _ForTestToggleOpenUrl(bool enable);
void setIgtkDialogRunReturnVal(gint val);
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Consider making setIgtkDialogRunReturnVal thread-safe.

Since setIgtkDialogRunReturnVal modifies a global variable, it should be made thread-safe to avoid race conditions.

#endif

} // namespace iptux
#endif // IPTUX_UIHELPER_H
10 changes: 10 additions & 0 deletions src/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ else
conf_data.set('SYSTEM_DARWIN', 0)
endif

config_debug = get_option('config-debug')
if config_debug.enabled()
config_debug_val = 1
elif config_debug.disabled()
config_debug_val = 0
else
config_debug_val = get_option('debug').to_int()
endif
conf_data.set('CONFIG_DEBUG', config_debug_val)

configure_file(
input: 'config.h.in',
output: 'config.h',
Expand Down
Loading