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

Sui(impl): Add chat list order configuration #421

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

Exagone313
Copy link
Contributor

@Exagone313 Exagone313 commented Jul 30, 2024

Adds a way to sort channels alphabetically in the sidebar

Notes for the review:

  • I am not quite sure about the naming
  • There are no checks done on the new sidebar-sort configuration value, but no other settings appear to have such checks. The default value I have provided in builtin.cfg is de facto arbitrary.
  • I wonder if there is a better way to access the configuration from list_sort_func
  • It's possible that I have missed some error checks to be done

@SilverRainZ
Copy link
Member

Sorry for replying so late, just fired by my employer recently :-P

data/builtin.cfg Outdated Show resolved Hide resolved
src/config/reader.c Outdated Show resolved Hide resolved
src/inc/sui/sui_config.h Outdated Show resolved Hide resolved
src/sui/sui_side_bar.c Outdated Show resolved Hide resolved
src/sui/sui_side_bar.c Outdated Show resolved Hide resolved
@Exagone313 Exagone313 changed the title Sui(impl): Add sidebar-sort configuration Sui(impl): Add chat list order configuration Aug 25, 2024
@Exagone313
Copy link
Contributor Author

@SilverRainZ I updated it.

@@ -16,5 +16,9 @@ void srn_application_config_free(SrnApplicationConfig *cfg){
}

SrnRet srn_application_config_check(SrnApplicationConfig *cfg){
if (g_strcmp0(cfg->ui->window.chat_list_order, CHAT_LIST_ORDER_RECENT) != 0
&& g_strcmp0(cfg->ui->window.chat_list_order, CHAT_LIST_ORDER_ALPHABET) != 0){
return SRN_ERR;
Copy link
Member

Choose a reason for hiding this comment

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

You can return RET_ERR(xxx") to provide a more friendly error message.

Copy link
Contributor Author

@Exagone313 Exagone313 Aug 26, 2024

Choose a reason for hiding this comment

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

@SilverRainZ Should I update the PO files after adding an error message?

Copy link
Member

Choose a reason for hiding this comment

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

It is fine, we can do this when we want to refresh our translation.

@Exagone313 Exagone313 marked this pull request as draft August 26, 2024 07:43
@Exagone313 Exagone313 marked this pull request as ready for review August 26, 2024 08:06
Copy link
Member

@SilverRainZ SilverRainZ left a comment

Choose a reason for hiding this comment

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

LGTM

@SilverRainZ SilverRainZ merged commit 8258011 into SrainApp:master Aug 26, 2024
2 checks passed
@Exagone313 Exagone313 deleted the sidebar-sort-alphabetically branch August 26, 2024 21:53
@SilverRainZ SilverRainZ added this to the 1.8 milestone Sep 5, 2024
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.

2 participants