Skip to content

Conversation

@AstreaTSS
Copy link
Member

Pull Request Type

  • Feature addition
  • Bugfix
  • Documentation update
  • Code refactor
  • Tests improvement
  • CI/CD pipeline enhancement
  • Other: [Replace with a description]

Description

This PR makes the autocomplete part of the slash command guide clearer. This basically resolves #1349's original complaint, though this doesn't change anything about AutocompleteContext's API reference.

Changes

  • Make it clearer what ctx.input_text is through a variable.
  • Note how to get other options in the command (through ctx.kwargs).
  • Adjust responding comment wording.

Related Issues

#1349

Test Scenarios

Python Compatibility

  • I've ensured my code works on Python 3.10.x
  • I've ensured my code works on Python 3.11.x

Checklist

  • I've run the pre-commit code linter over all edited files
  • I've tested my changes on supported Python versions
  • I've added tests for my code, if applicable
  • I've updated / added documentation, where applicable

Copy link
Contributor

@i0bs i0bs left a comment

Choose a reason for hiding this comment

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

I'm sorry, but I'm -1 for this. I don't see any benefit we get out of assigning ctx.input_text to a string_option_input variable. I think most developers will look at the already given example and use IntelliSense to determine what the input_text property method will do, including the docstring and type return.

I'm open to us maybe putting comments in the choice population instead, but I'm unable to find any good rationale for bringing this to our documentation.

@AstreaTSS
Copy link
Member Author

The idea is for the code to be understandable without an IDE or the like - admittedly, using ctx.input_text alone might make that clear, but I felt it couldn't hurt to add more clarification and be explicit rather than implicit.

I'm willing to change it if need be, but I genuinely don't see the harm in it.

@i0bs
Copy link
Contributor

i0bs commented Jul 3, 2023

The idea is for the code to be understandable without an IDE or the like [...]

Honestly, if developers aren't using IntelliSense for their Python projects when we already heavily rely on people to infer off of typings, docstrings, etc., that sounds like an L on their part. I think it'd be asinine if someone wasn't using a tool like that, especially when there's such prevalence for it among n/vim, JB, Atom and VSC editors.

@AstreaTSS
Copy link
Member Author

I disagree (at least in the context for a guide), but I think it's just better if someone else weighs in their opinion.

Copy link
Member

@silasary silasary left a comment

Choose a reason for hiding this comment

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

Seems like a good change to me

@LordOfPolls LordOfPolls merged commit 8b6415b into interactions-py:unstable Jul 7, 2023
@AstreaTSS AstreaTSS deleted the autocomplete-docs branch July 13, 2023 19:19
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.

4 participants