Skip to content

Update submit error message when submitting a directory #724

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.

Sign up for GitHub

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

Merged
nywilken merged 5 commits into from
Aug 31, 2018
Merged

Update submit error message when submitting a directory #724

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7654db
Update error message for submision of directory
Aug 29, 2018
91088b8
Update test to check for helpful error
Aug 29, 2018
2c58245
Updating message format to use BinaryName
Aug 29, 2018
7c03f28
Update error message format further
Aug 30, 2018
088037a
Fix test by double escaping
Aug 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cmd/submit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ func runSubmit(cfg config.Config, flags *pflag.FlagSet, args []string) error {

%s

Please provide the path to the file you wish to submit, e.g. : %s submit FILENAME
Copy link
Copy Markdown
Contributor

@nywilken nywilken Aug 30, 2018

Choose a reason for hiding this comment

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

One more minor nitpick. We generally put the example command on a separate line to avoid it being pushed off the terminal window (longer strings) and to make is easier for copying/pasting.

Can we update the message so that it reads and looks like the following.

    You are submitting a directory, which is not currently supported.

        /home/wilken/Development/exercism/vimscript

    Please change into the directory and provide the path to the file(s) you wish to submit

        exercism submit FILE1 FILE2

Note: I am making the assumption that if a user is trying to submit a directory they probably want to submit multiple files; hence the reason for calling out FILE1 FILE2. If that is not the case feel free to just keep FILENAME.

Copy link
Copy Markdown
Contributor Author

@sfairchild sfairchild Aug 30, 2018

Choose a reason for hiding this comment

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

The use case for this was a new student trying to submit hello_world for the very first time and not knowing how to fix their issue with just the line

You are submitting a directory, which is not currently supported.

Listing the multiple files might be helpful for letting users know that you can submit more than 1 file. I didn't even know this was an option until I dug into the source. My fear is listing multiple files will be confusing for someone who if very new. I am happy to make the change based on what everyone else thinks. @kytrinyx


    Please change into the directory and provide the path to the file(s) you wish to submit

        exercism submit FILE1 FILE2

vs


    Please change into the directory and provide the path to the file(s) you wish to submit

        exercism submit FILE

Copy link
Copy Markdown
Contributor

@nywilken nywilken Aug 30, 2018

Choose a reason for hiding this comment

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

Let’s drop the multiple files message. I agree that it can be confusing. Which is why I called out the note.

Copy link
Copy Markdown
Member

@NobbZ NobbZ Aug 30, 2018

Choose a reason for hiding this comment

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

It is a common convention to put optional arguments (named and positional) into square brackets, also using ellipsis to signal "or more", so that line should IMHO read exercism submit FILE1 [FILE2 ...]

Copy link
Copy Markdown
Member

@NobbZ NobbZ Aug 30, 2018

Choose a reason for hiding this comment

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

But I think, a single file example is sufficient if we extend the usage (exercism help submit) as described below and remind in the error that you can get more info there.

  1. be more explicit about the fact that at least one file is necessary
  2. be more explicit that one or more files can be specified

The current help output only states in the description that a list of files is needed, the actual usage does not mention it, but flags only as optional (square brackets).

Copy link
Copy Markdown
Contributor Author

@sfairchild sfairchild Aug 30, 2018

Choose a reason for hiding this comment

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

I agree with @NobbZ that the documentation for submit could be improved. Maybe a seperate issue could be opened to update that.

➜  ~ exercism version
exercism version 3.0.9
➜  ~ exercism submit -h
Submit your solution to an Exercism exercise.

	Call the command with the list of files you want to submit.

Usage:
   submit [flags]

Aliases:
  submit, s

Flags:
  -h, --help   help for submit

Global Flags:
      --timeout int   override the default HTTP timeout (seconds)
  -v, --verbose       verbose output

For now I updated the error message to

    Please change into the directory and provide the path to the file(s) you wish to submit

        exercism submit FILENAME

Copy link
Copy Markdown
Contributor

@nywilken nywilken Aug 30, 2018

Choose a reason for hiding this comment

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

@sfairchild seeing this now. Thanks for the update. I’ll open an issue for the help command, if you haven’t already, and please feel free to add any additional requirements for the usage details.


`
return fmt.Errorf(msg, arg)
return fmt.Errorf(msg, arg, BinaryName)
Copy link
Copy Markdown
Contributor

@nywilken nywilken Aug 30, 2018

Choose a reason for hiding this comment

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

👍

}

src, err := filepath.EvalSymlinks(arg)
Expand Down
1 change: 1 addition & 0 deletions cmd/submit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func TestSubmitFilesAndDir(t *testing.T) {
}
err = runSubmit(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), files)
assert.Regexp(t, "submitting a directory", err.Error())
assert.Regexp(t, "Please provide the path to the file you wish to submit", err.Error())
}

func TestSubmitFiles(t *testing.T) {
Expand Down