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

Replace usages of TYPED_EVENT with til::event #16837

Merged
merged 10 commits into from
Mar 20, 2024

Conversation

zadjii-msft
Copy link
Member

This PR automagically finds and replaces all1 usages of our TYPED_EVENT macro with til::event. Benefits include:

  • less macro magic
  • editors are more easily able to figure out the relationship between til::event<> Foo;, Foo.raise(...), and bar.Foo({this, &Bar::FooHandler}) (whereas before the relationship between _FooHandlers(...) and bar.Foo({...})) couldn't be figured out by vscode & sublime.

Other find & replace work that had to be done:

  • I added the til::typed_event<> == <IInspectable, IInspectable> thing from Refactor Pane to be able to host non-terminal content #16170, since that is goodness
  • I actually fixed til::property_changed_event, so you can use that for your property changed events. They're all the same anyways.
  • events had to come before WINRT_PROPERTYs, since the latter macro leaves us in a private: block
  • Pane::SetupChildCloseHandlers I had to swap back, because the script thought that was an event 🤦
  • ProfileViewModel::DeleteProfile had to be renamed DeleteProfileRequested, since there was already a DeleteProfile method on it.
  • WindowManager.cpp was directly wiring up it's winrt::events to the monarch & peasant. That doesn't work with til::events and I'm kinda surprised it ever did
The script in question
import os
import re

def replace_in_file(file_path, file_name):
    with open(file_path, 'r', encoding="utf8") as file:
        content = file.read()
    
    found_matches = False

    # Define the pattern for matching
    pattern = r' WINRT_CALLBACK\((\w+),\s*(.*?)\);'
    event_matches = re.findall(pattern, content)
    if event_matches:
        found_matches = True

        print(f'found events in {file_path}:')
        for match in event_matches:
            name = match[0]
            args = match[1]

            if name == "newConnection" and file_name == "ConptyConnection.cpp":
                # This one is special
                continue

            old_declaration = 'WINRT_CALLBACK(' + name + ', ' + args + ');'
            new_declaration = 'til::event<' + args + '> ' + name + ';' if name != "PropertyChanged" else 'til::property_changed_event PropertyChanged;'
            print(f'  {old_declaration} -> {new_declaration}')
            content = content.replace(old_declaration, new_declaration)


    typed_event_pattern = r' TYPED_EVENT\((\w+),\s*(.*?)\);'
    typed_matches = re.findall(typed_event_pattern, content)
    if typed_matches:
        found_matches = True
        print(f'found typed_events in {file_path}:')
        for match in typed_matches:
            name = match[0]
            args = match[1]

            if name == "newConnection" and file_name == "ConptyConnection.cpp":
                # This one is special
                continue

            old_declaration = f'TYPED_EVENT({name}, {args});'
            was_inspectable = (args == "winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable" ) or (args == "IInspectable, IInspectable" )
            new_declaration = f'til::typed_event<{args}> {name};' if not was_inspectable else f"til::typed_event<> {name};"
            print(f'  {old_declaration} -> {new_declaration}')
            content = content.replace(old_declaration, new_declaration)

    handlers_pattern = r'_(\w+)Handlers\('
    handler_matches = re.findall(handlers_pattern, content)
    if handler_matches:
        found_matches = True
        print(f'found handlers in {file_path}:')
        for match in handler_matches:
            name = match

            if name == "newConnection" and file_name == "ConptyConnection.cpp":
                # This one is special
                continue

            old_declaration = f'_{name}Handlers('
            new_declaration = f'{name}.raise('
            print(f'  {old_declaration} -> {new_declaration}')
            content = content.replace(old_declaration, new_declaration)


    if found_matches:
        with open(file_path, 'w', encoding="utf8") as file:
            file.write(content)

def find_and_replace(directory):
    for root, dirs, files in os.walk(directory):
        if 'Generated Files' in dirs:
            dirs.remove('Generated Files')  # Exclude the "Generated Files" directory

        for file in files:
            if file.endswith('.cpp') or file.endswith('.h') or file.endswith('.hpp'):
                file_path = os.path.join(root, file)
                try:
                    replace_in_file(file_path, file)
                except Exception as e:
                    print(f"error reading {file_path}")
                    if file == "TermControl.cpp":
                        print(e)
                    # raise e

# Replace in files within a specific directory
directory_path = 'D:\\dev\\public\\terminal\\src'
find_and_replace(directory_path)

Footnotes

  1. there are other macros we use that were also using this macro, those weren't replaced.

zadjii-msft and others added 3 commits March 7, 2024 06:46
src/cascadia/TerminalApp/AboutDialog.h
src/cascadia/TerminalApp/CommandPalette.cpp
src/cascadia/TerminalApp/CommandPalette.h
src/cascadia/TerminalApp/FilteredCommand.h
src/cascadia/TerminalApp/HighlightedText.h
src/cascadia/TerminalApp/SuggestionsControl.cpp
src/cascadia/TerminalApp/SuggestionsControl.h
src/cascadia/TerminalApp/TabHeaderControl.cpp
src/cascadia/TerminalApp/TabHeaderControl.h
src/cascadia/TerminalApp/TabRowControl.h
src/cascadia/TerminalApp/TerminalPage.cpp
src/cascadia/TerminalApp/TerminalPage.h
src/cascadia/TerminalApp/TerminalTabStatus.h
src/cascadia/TerminalConnection/ConptyConnection.cpp
src/cascadia/TerminalConnection/ConptyConnection.h
src/cascadia/TerminalConnection/EchoConnection.cpp
src/cascadia/TerminalConnection/EchoConnection.h
src/cascadia/TerminalSettingsEditor/Appearances.cpp
src/cascadia/TerminalSettingsEditor/Appearances.h
src/cascadia/TerminalSettingsEditor/ColorSchemes.h
src/cascadia/TerminalSettingsEditor/EditColorScheme.h
src/cascadia/TerminalSettingsEditor/EnumEntry.h
src/cascadia/TerminalSettingsEditor/MainPage.h
src/cascadia/TerminalSettingsEditor/Profiles_Advanced.h
src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h
src/cascadia/TerminalSettingsEditor/Profiles_Base.h
src/cascadia/TerminalSettingsEditor/ViewModelHelpers.h
src/cascadia/WindowsTerminal/IslandWindow.h
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

I didn't see it here, but can we also remove the definition for TYPED_EVENT and WINRT_EVENT?

@zadjii-msft
Copy link
Member Author

there are other macros we use that were also using this macro, those weren't replaced

src/tools/scratch/main.cpp Outdated Show resolved Hide resolved
@DHowett
Copy link
Member

DHowett commented Mar 8, 2024

the PR still doesn't compile 🙃

@zadjii-msft zadjii-msft merged commit a971663 into main Mar 20, 2024
20 checks passed
@zadjii-msft zadjii-msft deleted the dev/migrie/til-events-for-all branch March 20, 2024 16:02
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.

3 participants