Skip to content

Conversation

@deathaxe
Copy link
Member

ST4 has been shipping sublime.RegionFlags for quite a while.

Maybe worth to switch from 3rd-party to built-in solution.

Note: requires ST4135+.

ST4 has been shipping `sublime.RegionFlags` for quite a while.
Time to switch from 3rd-party to built-in solution.
Copy link
Member

@FichteFoll FichteFoll left a comment

Choose a reason for hiding this comment

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

Thanks. I'll leave this unmerged until the next release to remind myself to adjust the release channel for the ST build requirement.

Example:
flags: sublime.RegionFlags = region_flags_from_strings(["DRAW_EMPTY", "DRAW_NO_FILL"])
"""
return RegionFlags(sum(getattr(RegionFlags, n, RegionFlags.NONE) for n in flag_names))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return RegionFlags(sum(getattr(RegionFlags, n, RegionFlags.NONE) for n in flag_names))
return sum(getattr(RegionFlags, n, RegionFlags.NONE) for n in set(flag_names))

Isn't the outer RegionFlags call redundant since we're already getting and summing RegionFlags "instances"? Also wrapping in a set to deduplicate the strings and prevent unintended side effects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Runtime yes, typechecker/linter no. Without that pyright/basedpyright complains about any assignments.

Copy link
Member

@FichteFoll FichteFoll Jan 22, 2026

Choose a reason for hiding this comment

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

I wonder if they can they handle functools.reduce(operator.or_, (getattr(RegionFlags, n, RegionFlags.NONE) for n in flag_names), RegionFlags.NONE).

Copy link
Member Author

@deathaxe deathaxe Jan 22, 2026

Choose a reason for hiding this comment

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

It technically works, but looks more noisy with more dependencies involved and is 2.5x slower.

It's however an ideal example of how "modern" python bloats and overcomplicates simple tasks undermining any attempts by core team to improve performance.

Copy link
Member

@FichteFoll FichteFoll Jan 22, 2026

Choose a reason for hiding this comment

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

Yes, this was not meant as a change request. I was just thinking out loud.

However, the set issue is still open because currently trying to resolve region_flags_from_strings(["DRAW_EMPTY", "DRAW_EMPTY"]) will have very undesired effects. Using reduce(operator.or_, …) would address this, but so would wrapping the iterable in a set(…).

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the set issue?

The verbose way to implement the function using official RegionFlags semantics would be ...

def region_flags_from_strings(flag_names):
    result = RegionFlags.NONE
    for name in flag_names:
        try:
            result |= RegionFlags[name]
        except KeyError:
            pass

    return result

But that's still 2x slower.


import functools
import operator
from sublime import RegionFlags
import timeit

def merge_sum(flag_names: list[str]) -> RegionFlags:
    return sum(getattr(RegionFlags, n, RegionFlags.NONE) for n in flag_names)

def merge_sum_cast(flag_names: list[str]) -> RegionFlags:
    return RegionFlags(sum(getattr(RegionFlags, n, RegionFlags.NONE) for n in flag_names))

def merge_reduce(flag_names: list[str]) -> RegionFlags:
    return functools.reduce(operator.or_, (getattr(RegionFlags, n, RegionFlags.NONE) for n in flag_names), RegionFlags.NONE)

def region_flags_from_strings(flag_names: list[str]) -> RegionFlags:
    result = RegionFlags.NONE
    for name in flag_names:
        try:
            result |= RegionFlags[name]
        except KeyError:
            pass
    return result

print(timeit.timeit(lambda: merge_sum(["DRAW_EMPTY", "HIDE_ON_MINIMAP"])))
# > 0.34730920000583865
print(timeit.timeit(lambda: merge_sum_cast(["DRAW_EMPTY", "HIDE_ON_MINIMAP"])))
# > 0.601449900001171
print(timeit.timeit(lambda: merge_reduce(["DRAW_EMPTY", "HIDE_ON_MINIMAP"])))
# > 1.6256988999957684
print(timeit.timeit(lambda: region_flags_from_strings(["DRAW_EMPTY", "HIDE_ON_MINIMAP"])))
# > 1.3742471000005025

Copy link
Member

Choose a reason for hiding this comment

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

What's the set issue?

RegionFlags.DRAW_EMPTY + RegionFlags.DRAW_EMPTY != RegionFlags.DRAW_EMPTY | RegionFlags.DRAW_EMPTY

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants