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

[Docs] Make Getting started guide explain use of quotes for arguments with spaces #3555

Merged
merged 5 commits into from Feb 21, 2020

Conversation

Dav-Git
Copy link
Contributor

@Dav-Git Dav-Git commented Feb 15, 2020

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

This closes #3111

@Dav-Git Dav-Git requested review from palmtree5 and tekulvw as code owners Feb 15, 2020
@@ -351,6 +351,9 @@ The cog guides are formatted the same. They're divided into 3 sections:

Arguments enclosed in ``[ ]`` means that the argument is **optional**
for the command; you can decide to use it or not.

If your argument includes spaces like ``Hello world!`` you need to place
Copy link
Contributor

@Drapersniper Drapersniper Feb 15, 2020

Choose a reason for hiding this comment

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

Hmmmm this is not true for every command though... so generelizing here can leave to confusion.

Copy link
Contributor Author

@Dav-Git Dav-Git Feb 15, 2020

Choose a reason for hiding this comment

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

I'm not aware of any differantiation. How could the end user tell they need or don't need quotes?

@jack1142 jack1142 changed the title [Docs] getting_started.rst now explains use of quotes for arguments with spaces [Docs] Make Getting started guide explain use of quotes for arguments with spaces Feb 15, 2020
@jack1142 jack1142 added the Category: Docs label Feb 15, 2020
@Dav-Git
Copy link
Contributor Author

@Dav-Git Dav-Git commented Feb 21, 2020

Any other thoughts on this?

@jack1142
Copy link
Member

@jack1142 jack1142 commented Feb 21, 2020

There's no differentiation between the arguments that require quotes and those that don't, but the only arguments that may not require quotes are the last arguments.

It is a convention of sorts to not require quotes for last argument, however it is not used in all commands (probably even some commands in core cogs may not use the "consume all" for last argument) and also, if last argument is shown in usage with ellipsis (e.g. <argument>..., [argument...]) then the quotes are gonna be required for last argument as there's no defined amount of arguments.

Probably the things I said should be somehow nicely explained in the guide even with the limited knowledge you can get from command's usage. IMO, saying last argument usually doesn't require quotes around it + maybe mentioning ellipsis would be good enough, we can't make this perfect because usage of a command doesn't show any difference between consume all and regular argument.

Also, while I think it wouldn't be a bad idea to have some indicator in usage when last argument doesn't require spaces, that would have to be a separate discussion (probably not an easy one).

@Dav-Git
Copy link
Contributor Author

@Dav-Git Dav-Git commented Feb 21, 2020

Yeah, okay so we're pretty much where we left off. Just for the sake of people not unintentionally not using quotes wouldn't it be better to say they are always required instead of having a "maybe eventually in some cases" in a guide that's meant for absolute beginners? I'd personally leave it as is but if you think it needs to be changed before this can be approved I'll try to find a good explanation.

@Drapersniper
Copy link
Contributor

@Drapersniper Drapersniper commented Feb 21, 2020

It may not be always required though if I have a consume all and add quotes ... things .. mess up

@synrg
Copy link

@synrg synrg commented Feb 21, 2020

Re. quotes "mess up", I have at least one consume all command on a cog of mine that addresses that very thing: if users expect quotes to work, especially if the entity is one that in other commands does require quotes, then the command should deal with the possibility the user typed them even though they're not required. So I wrote the code to do that. This is in line with my feeling users will learn "quotes are required" as a general rule anyway, even though in some corner cases they may not be.

@Dav-Git
Copy link
Contributor Author

@Dav-Git Dav-Git commented Feb 21, 2020

.

@jack1142
Copy link
Member

@jack1142 jack1142 commented Feb 21, 2020

I am happy with this considering what we can do with how the UX shows it.

Drapersniper
Drapersniper previously approved these changes Feb 21, 2020
Copy link
Contributor

@Drapersniper Drapersniper left a comment

LGTM

@jack1142 jack1142 added the Type: Enhancement label Feb 21, 2020
@jack1142 jack1142 added this to the 3.3.2 milestone Feb 21, 2020
docs/getting_started.rst Outdated Show resolved Hide resolved
Co-Authored-By: Flame442 <34169552+Flame442@users.noreply.github.com>
@Dav-Git
Copy link
Contributor Author

@Dav-Git Dav-Git commented Feb 21, 2020

Wait how did I dismiss something?

@jack1142
Copy link
Member

@jack1142 jack1142 commented Feb 21, 2020

That's normal, you dismissed Draper's approving review cause you made changes since he made the review. Nothing to worry :)

@Dav-Git
Copy link
Contributor Author

@Dav-Git Dav-Git commented Feb 21, 2020

Ah I see.

@jack1142 jack1142 merged commit 106804a into Cog-Creators:V3/develop Feb 21, 2020
5 checks passed
@Dav-Git Dav-Git deleted the patch-1 branch Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Docs Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants