Astal icon indicating copy to clipboard operation
Astal copied to clipboard

Add Uncrustify.cfg for formatting

Open ARKye03 opened this issue 1 year ago • 2 comments

[!WARNING] There is a bug with uncrustify in lib/apps/application.vala line 59 (after formatting), that deletes the ;, adding it back after formatting the file will work as expected and no more issues should arise.

Add uncrustify for consistent formatting rules across Vala and C files. Vala example result: lib/apps/fuzzy.vala

from
namespace AstalApps {
private int fuzzy_match_string(string pattern, string str) {
    const int unmatched_letter_penalty = -1;
    int score = 100;

    if (pattern.length == 0) return score;
    if (str.length < pattern.length) return int.MIN;

    bool found = fuzzy_match_recurse(pattern, str, score, true, out score);
    score += unmatched_letter_penalty * (str.length - pattern.length);
    
    if(!found) score = -10;

    return score;
}

private bool fuzzy_match_recurse(string pattern, string str, int score, bool first_char, out int result) {
    result = score;
    if (pattern.length == 0) return true;

    int match_idx = 0;
    int offset = 0;
    unichar search = pattern.casefold().get_char(0);
    int best_score = int.MIN;

    while ((match_idx = str.casefold().substring(offset).index_of_char(search)) >= 0) {
        offset += match_idx;
        int subscore;
        bool found = fuzzy_match_recurse(
            pattern.substring(1),
            str.substring(offset + 1),
            compute_score(offset, first_char, str, offset), false, out subscore);
        if(!found) break;
        best_score = int.max(best_score, subscore);
        offset++;
    }
    
    if (best_score == int.MIN) return false;
    result += best_score;
    return true;
}

private int compute_score(int jump, bool first_char, string match, int idx) {
    const int adjacency_bonus = 15;
    const int separator_bonus = 30;
    const int camel_bonus = 30;
    const int first_letter_bonus = 15;
    const int leading_letter_penalty = -5;
    const int max_leading_letter_penalty = -15;

    int score = 0;

    if (!first_char && jump == 0) {
        score += adjacency_bonus;
    }
    if (!first_char || jump > 0) {
        if (match[idx].isupper() && match[idx-1].islower()) {
            score += camel_bonus;
        }
        if (match[idx].isalnum() && !match[idx-1].isalnum()) {
            score += separator_bonus;
        }
    }
    if (first_char && jump == 0) {
        score += first_letter_bonus;
    }
    if (first_char) {
        score += int.max(leading_letter_penalty * jump, max_leading_letter_penalty);
    }

    return score;
}
}
to
namespace AstalApps {
private int fuzzy_match_string(string pattern, string str) {
	const int unmatched_letter_penalty = -1;
	int score = 100;

	if (pattern.length == 0) {
		return score;
	}
	if (str.length < pattern.length) {
		return int.MIN;
	}

	bool found = fuzzy_match_recurse(pattern, str, score, true, out score);
	score += unmatched_letter_penalty * (str.length - pattern.length);

	if (!found) {
		score = -10;
	}

	return score;
}

private bool fuzzy_match_recurse(string pattern, string str, int score, bool first_char, out int result) {
	result = score;
	if (pattern.length == 0) {
		return true;
	}

	int match_idx = 0;
	int offset = 0;
	unichar search = pattern.casefold().get_char(0);
	int best_score = int.MIN;

	while ((match_idx = str.casefold().substring(offset).index_of_char(search)) >= 0) {
		offset += match_idx;
		int subscore;
		bool found = fuzzy_match_recurse(
			pattern.substring(1),
			str.substring(offset + 1),
			compute_score(offset, first_char, str, offset), false, out subscore);
		if (!found) {
			break;
		}
		best_score = int.max(best_score, subscore);
		offset++;
	}

	if (best_score == int.MIN) {
		return false;
	}
	result += best_score;
	return true;
}

private int compute_score(int jump, bool first_char, string match, int idx) {
	const int adjacency_bonus = 15;
	const int separator_bonus = 30;
	const int camel_bonus = 30;
	const int first_letter_bonus = 15;
	const int leading_letter_penalty = -5;
	const int max_leading_letter_penalty = -15;

	int score = 0;

	if (!first_char && jump == 0) {
		score += adjacency_bonus;
	}
	if (!first_char || jump > 0) {
		if (match[idx].isupper() && match[idx - 1].islower()) {
			score += camel_bonus;
		}
		if (match[idx].isalnum() && !match[idx - 1].isalnum()) {
			score += separator_bonus;
		}
	}
	if (first_char && jump == 0) {
		score += first_letter_bonus;
	}
	if (first_char) {
		score += int.max(leading_letter_penalty * jump, max_leading_letter_penalty);
	}

	return score;
}
}

Summary

  • Indentation based on tabs
  • Unindented namespaces
  • No space before parens on function declarations
  • Keywords like if, else, catch, etc will have space before parens.

A full showcase of all formatted files can be seen at https://github.com/ARKye03/astal/tree/refactor-style

[!NOTE] I ran fd -e c -e h -e vala . | xargs uncrustify -c uncrustify.cfg --no-backup for it

ARKye03 avatar Nov 11 '24 15:11 ARKye03

I assume you haven't update the showcases, I'm getting different results, so I'll just refer to files by name

  1. in apps/application.vala in to_json the builder chaining has an extra indent.
  2. lambdas have an extra indent for example /gtk3/src/widget/widget.vala, but apparently not everywhere? for example in network/wifi.vala` it looks good
widget.destroy.connect(() => {
        remove_provider(widget);
    });
  1. lambdas again in gtk3/src/application.vala, but this time one less indent
Bus.own_name(
    BusType.SESSION,
    application_id,
    BusNameOwnerFlags.NONE,
    (conn) => {
    try {
        this.conn = conn;
        conn.register_object("/io/Astal/Application", this);
    } catch (Error err) {
        critical(err.message);
    }
},
    () => {},
    () => {}
    );
  1. closing parens also have an extra indent like above

  2. closing parens again in function/method declaration, for example greet/client.vala

public async void login(
	string username,
	string password,
	string cmd
	) throws GLib.Error {
	yield login_with_env(username, password, cmd, {});
}
  1. ternaries should be indented mpris/player.vala in shuffle method for example
shuffle_status = shuffle_status == Shuffle.ON
? Shuffle.OFF
: Shuffle.ON;

We should also stay with spaces instead of tabs

Aylur avatar Nov 13 '24 00:11 Aylur

  1. in apps/application.vala in to_json the builder chaining has an extra indent.

I can't find a single solution for this behaviour. The required rule for it is indent_member, which should have a value of 0 or 1 or the same as indent_column. However, neither achieves what is wanted. Instead of 4 spaces, 7 arise. I'll keep investigating if a rule unconsciously changes this, but I tried every reasonable rule for it. You can increase the indent, not lower it. I opened an issue for this particular scenario https://github.com/uncrustify/uncrustify/issues/4396.


  1. lambdas have an extra indent for example /gtk3/src/widget/widget.vala, but apparently not everywhere? for example in network/wifi.vala` it looks good

This can be fixed by setting indent_class = FALSE, but this would mean no indentation for classes, if we want to keep indentation for classes and still indent lambda body, this can be set indent_paren_open_brace = TRUE, however, this will provoke probably unwanted behaviour, where every lambda gets indented like:

activate.connect(() => {
                        var display = Gdk.Display.get_default();
                        display.monitor_added.connect((mon) => {
                                                        monitor_added(mon);
                                                        notify_property("monitors");
                                                    });
                        display.monitor_removed.connect((mon) => {
                                                            monitor_removed(mon);
                                                            notify_property("monitors");
                                                        });
                    });

  1. closing parens also have an extra indent like above
  2. closing parens again in function/method declaration, for example greet/client.vala

It was fixed by setting indent_paren_close = 2 where 2 means Indent to the brace level, new result:

Bus.own_name(
    BusType.SESSION,
    application_id,
    BusNameOwnerFlags.NONE,
    (conn) => {
        try {
            this.conn = conn;
            conn.register_object("/io/Astal/Application", this);
        } catch (Error err) {
            critical(err.message);
        }
    },
    () => {},
    () => {}
);
public async void login(
    string username,
    string password,
    string cmd
) throws GLib.Error {
    yield login_with_env(username, password, cmd, {});
}

  1. ternaries should be indented mpris/player.vala in shuffle method for example

After setting pos_conditional = break it behaves as needed:

public void shuffle() {
    if (shuffle_status == Shuffle.UNSUPPORTED) {
        critical(@"shuffle is unsupported by $bus_name");
        return;
    }

    shuffle_status = shuffle_status == Shuffle.ON
                     ? Shuffle.OFF
                     : Shuffle.ON;
}

We should also stay with spaces instead of tabs

Now it uses spaces

ARKye03 avatar Nov 13 '24 04:11 ARKye03

I ended up going through the default config and setting it up. It was pure pain.

If I do end up working with Vala a lot more I'll make a Vala dedicated formatter because out of the 850 options half of them dont have a reason to exist. Who would need to configure the braces of a try catch finally expression differently for the three keywords? I'm also just simply not willing to use elementary's formetter because having a space before parens is bullshit.

Aylur avatar Oct 01 '25 21:10 Aylur