exercism/cli

Allow submitting solution without specifying files

ErikSchierboom opened this issue · 24 comments

ErikSchierboom commented

One of the most requested features for the CLI is to be able to submit a solution without having to explicitly specify the files to submit. With Exercism v3, each exercise will have a .meta/config.json file (see https://github.com/exercism/docs/blob/main/building/tracks/concept-exercises.md#file-metaconfigjson and https://github.com/exercism/docs/blob/main/building/tracks/practice-exercises.md#file-metaconfigjson). This file contains a files property with a nested a solution string array property. The values in the solution array are the names of the files that the student should work on. In other words: these file names can be used to determine the files that should be submitted, provided no files were specified by the student.

As an example, consider the following .meta/config.json file:

{
  "files": {
    "solution": ["Lasagna.fs"],
    "test": ["LasagnaTests.fs"],
    "exemplar": [".meta/Exemplar.fs"]
  }
}

If the student runs exercism submit (no file names specified), the CLI should then interpret this as if the student ran exercism submit Lasagna.fs.

Note that if a student does specify files, the CLI should use those and ignore the .meta/config.json file.

Also note that the solution property is an array, so it can contain multiple files (which should all be submitted).

andrerfcsantos commented

I'd like to work on this!

  • 👍4
  • ❤️1
andrerfcsantos commented

I took a look at the code and I think I have a good idea where to make the changes. I have one question though.

About the .meta/config.json file - since it is in the metadata folder and "not presented to the user" I assume this means that it will not be shipped when the student downloads the exercise and we must grab this file from the repository. Am I correct in this assumption? If so, should we fetch the file when the students tries to submit the solution and put it somewhere for future use like the .exercism folder in each exercise, or should we fetch .meta/config.json every time the student tries to submit?

ErikSchierboom commented

Good question. We definitely want the file to be present locally. Maybe the .exercism folder would be a good location for that. @iHiD any thoughts?

haguro commented

Would it make more sense to fetch config.json upon download rather than wait until the user submits the solution? Though naturally upon release existing users won't have the file at all so a fallback for it to be fetched by submit instead might be necessary anyway.

iHiD commented

It should definitely be downloaded with the exercise IMO. The issue, of course, is that for many historic solutions it won't be there. So I'd say download it and store it in .exercism, and if at submit time it is missing, download it then too.

Rustery commented

I think there should be a link to #975 issue with the same question.

andrerfcsantos commented

It should definitely be downloaded with the exercise IMO. The issue, of course, is that for many historic solutions it won't be there. So I'd say download it and store it in .exercism, and if at submit time it is missing, download it then too.

Sounds good, will implement like this.

One additional question: currently I'm grabbing the exercise config file from the track repo, but looking at the code it seems we are getting most of the info (including other config files) from the API. Is getting the file directly from the repo the intended way to do this. Is there (or will be) an API endpoint for this file?

Related to the previous question: if we do get the file from the repository, the link to the .meta file depends on if the exercise is a concept exercise or concept (e.g https://github.com/exercism/go/blob/main/exercises/practice/accumulate/.meta/config.json, note the /practice/ part of the link), which means that we also must know the kind of exercise, which I guess we can know from the config.json in the track repos (e.g https://github.com/exercism/go/blob/main/config.json). Should we also grab this file? Can we know this via the API?

iHiD commented

The API should have this file added to its list of sent files. I think it already does for v3 (@ErikSchierboom?) We don't use GitHub as a production source for anything.

ErikSchierboom commented

@andrerfcsantos @iHiD and I have discussed this, and this is what we decided upon:

  1. The API will return the .meta/config.json file along with the exercise files
  2. The CLI should downloaded any files in the .meta directory to the .exercism directory

This means that when an exercise is downloaded, there should be a .exercism/config.json file on the student's file system, not an .meta/config.json file. Makes sense?

See https://github.com/exercism/website/pull/1002/files for the corresponding PR.

ErikSchierboom commented

@andrerfcsantos Small bump regarding the above question: could you indicate what the status of this issue is?

andrerfcsantos commented

Hi @ErikSchierboom

It has been a couple of busy weeks and I hadn't the chance to look further into this issue.

Starting next week I'll have more free time and I plan to look into this issue.

  • 👍1
jodihamann commented

I'm happy to pick up this issue if it's stalled out. If so, I had a quick question about the desired implementation.

At the moment, the CLI is validating that files exist and that the path is not a directory returning the following message in the latter case:

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

    %s

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

    %s submit FILENAME

If the CLI is changed to allow submitting without specifying files, effectively submitting the current directory, should it also support submitting arbitrary directories?

andrerfcsantos commented

@ErikSchierboom I'm happy to hand over this issue to @jodihamann

  • 👍1
ErikSchierboom commented

@andrerfcsantos Great!

If the CLI is changed to allow submitting without specifying files, effectively submitting the current directory, should it also support submitting arbitrary directories?

@jodihamann Note that this change is not about submitting the current directory, but about automatically submitting a pre-defined set of files (namely those in the files.solution key in the .exercism/config.json file). This is the behavior that we want:

  1. If someone does exercism submit (so without any file names specified), the CLI then
  2. checks if a .exercism/config.json file exists,
  3. if this file exists, it reads its files.solution property
  4. if all the files from the files.solution property exist on disk, internally the command to execute is rewritten to exercism submit <files from files.solution> (the student wouldn't know about this, this is all internal)

We need to consider the following edge conditions:

  • The .exercism/config.json file does not exist. This happens if someone is using an older version of the CLI or a previously downloaded exercise. In this case we should do what we do at the moment, which is show an error message.
  • The exercism.config.json file does not have a files.solution key. This should not happen, but we should guard against this anyway. In this case we should do what we do at the moment, which is show an error message.
  • The exercism.config.json file has a files.solution key but it is an empty array. This should not happen, but we should guard against this anyway. In this case we should do what we do at the moment, which is show an error message.
  • The exercism.config.json file has a files.solution key, but it is not an array of string. This should not happen, but we should guard against this anyway. In this case we should do what we do at the moment, which is show an error message.
  • The exercism.config.json file has a files.solution key, but one or more files specified in this key don't exist on the file system. This is not very likely, but could happen. I think the simplest solution is to treat this as a mismatch and revert to the current behavior and treat this as an error.

TL;DR If the student runs exercism submit, and there is a .exercism/config.json file, and its files.solution key is non-empty, and all files listed in that key exist on the file system, then we should internally treat this as a call to exercism submit <files from files.solution>.

  • 👍4
coriolinus commented

Two feature requests:

  • Support git-style globs. This enables proper handling even if a student breaks out a part of their work into a separate file.
  • Support a fallback to a key in the track-level config.json if files.solution does not exist in the per-exercise config.json. For Rust, and likely for other languages, the set of default files to submit doesn't actually change per exercise. Of the two, this is the more desirable feature.

Given the second feature and the first, Rust would have very robust support by defining ["Config.toml", "Config.lock", "src/**.rs"] at the track level. Given the second feature and not the first, we'd still have decent support by defining ["Config.toml", "Config.lock", "src/lib.rs"], though it would be more fragile in case a student created a module in a new file.

Without the second feature, we have to copy this bit of boilerplate into the configuration of every exercise, which is tedious.

I don't have any particular opinion about where the exercism tool should cache the track-level config.json locally.

asarkar commented

+1. Cherrypicking individual files is prone to mistake. I should be able to do exercism submit <exercise_dir> from anywhere, since the CLI already knows the workspace path, or exercism submit, in which case, the CLI should consider the current directory as an exercise.

andrerfcsantos commented

I'll try to take a look at this again, now that I'm more familiar with the codebase.

I noticed there's already a PR created before this issue that attempts to address this: #929

The algorithm used there is different than the one discussed here and is before v3. Should I create a new PR if I get to look at this?

Support git-style globs. This enables proper handling even if a student breaks out a part of their work into a separate file.

This can be done easily.

Support a fallback to a key in the track-level config.json if files.solution does not exist in the per-exercise config.json. For Rust, and likely for other languages, the set of default files to submit doesn't actually change per exercise. Of the two, this is the more desirable feature.

My question here is how exactly we would get the track-level config. It'd be probably best if this config is provided by the API, but I'm not sure there's an endpoint for this. Grabbing the file directly from github doesn't seem a good idea, since the link contains the repo name and a branch name, both of which can change.

coriolinus commented
andrerfcsantos commented

That's what I had in mind, but I thought you were suggesting checking the track-level config as an additional fallback for not having .meta/config.json with a files.solution.

ErikSchierboom commented

but I thought you were suggesting checking the track-level config as an additional fallback for not having .meta/config.json with a files.solution.

We don't support files.solution to be empty (which is verified by configlet), so that fallback is not needed/supported.

Other than that: getting this functionality merged would be huge!

The algorithm used there is different than the one discussed here and is before v3. Should I create a new PR if I get to look at this?

I think that would make the most sense.

  • 👍2
andrerfcsantos commented

Created a PR: #1044

Feel free to take a look. Didn't include the "glob" feature because I think we need to discuss it a bit more and it's probably best if this feature is as self-contained as possible.

  • ❤️2
ErikSchierboom commented

Feel free to take a look. Didn't include the "glob" feature because I think we need to discuss it a bit more and it's probably best if this feature is as self-contained as possible.

Good call. I don't think we need this at the moment. Maybe for some time in the future, but then we'd also have to update the website to handle it.

coriolinus commented

Follow-up issue for globs subfeature: exercism/exercism#6399

coriolinus commented