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

It would be nice if PromptRequiredError already had the suggestions for the prompt #6

Open
Gnuxie opened this issue Sep 10, 2024 · 2 comments

Comments

@Gnuxie
Copy link
Member

Gnuxie commented Sep 10, 2024

  it("Will prompt for the policy list when it is missing", async function () {
    const { protectedRoomsSet, policyRoomManager } = await createProtectedRooms();
    const banResult = await CommandExecutorHelper.parseAndInvoke(
      DraupnirBanCommand,
      {
        policyRoomManager,
        roomResolver,
        issuerManager: protectedRoomsSet.issuerManager,
        defaultReasons: ["spam"],
        clientUserID: `@draupnir:ourserver.example.com` as StringUserID,
      },
      {},
      MatrixUserIDPresentationType.wrap(MatrixUserID.fromUserID("@spam:spam.example.com" as StringUserID)),
      undefined, // no policy room provided, we expect a prompt.
    );
    if (isOk(banResult)) {
      throw new TypeError(`We expect a prompt error to be returned`);
    }
    if (!(banResult.error instanceof PromptRequiredError)) {
      throw new TypeError(`Expected to have a prompt required error`)
    }
    expect(banResult.error.parameterRequiringPrompt.name).toBe("list");
  });

Would mean a test like this could also test that the prompt had been called properly.

@Gnuxie
Copy link
Member Author

Gnuxie commented Sep 10, 2024

Ok, it turns out that we need to do this now anyways, because stream.isPromptable() will never work as we keep recreating streams out of thin air in the parsing code.

We will call the prompt whenever one is available and show the suggestions in the error message, much like the An argument for the parameter ${parameter.name} was expected but was not provided.

@Gnuxie
Copy link
Member Author

Gnuxie commented Sep 10, 2024

Ok, the problem with doing that is it means we would have to pass the executor context all the way through to the parsing code which will be ugly. So I think we just change to PromptRequiredError by default and then something else can call them later.

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

No branches or pull requests

1 participant