Init exercise script #389

coriolinus · 2017-11-07T16:39:09Z

Add a script which generates an exercise stub. Closes #280.

Generates all required files and formatting
Generates README.md
Generates test cases if the exercise has a properly configured canonical-data.json
Interactively configures config.json

This should make it easier for first-time contributors to jump in, hopefully.

The goal is to ease contribution by setting up a project template and configuring tests from the canonical data, if any. Would-be contributors should be able to simply run this script and configure a few values in order to add a new exercise.

petertseng

There are too many unrelated changes in the README. I would not include the unrelated README changes, only the (I believe) one actual addition (instructions for adding the script). Otherwise this PR is doing more than what it claims to do.

petertseng · 2017-11-08T03:29:10Z

bin/init_exercise.py

+
+This module requires Python3.5 or newer
+"""
+from __future__ import print_function


My understanding is that this will have no effect on Python 3, but will allow this script to run and print its error messages on Python 2, when it instead would not be able to.

If this guess was correct, then there is no need to respond to it.

petertseng · 2017-11-08T03:35:14Z

bin/init_exercise.py

+    sys.exit(1)
+
+# check version info
+if sys.version_info[0] != 3 or sys.version_info[1] < 5:


I would voice some concern here about "But what if Python 4 comes out someday and this script works on it without modification!" but I guess this line can be dealt with at that point in the far future, if it still exists when Python 4 comes.

I thought that through when writing the line. My reasoning: Python3 included breaking changes from Python2. If there is ever a Python4 in the future, we can expect it to include some changes which break Python3 scripts anyway. This line simply ensures that, for this script at least, someone takes a look at exactly what the breaking changes are and whether things are affected before someone can run this and have things explode in their face.

petertseng · 2017-11-08T03:36:26Z

bin/init_exercise.py

+    return sp.stdout.strip()
+
+
+REPO_ROOT = output_of('git rev-parse --show-toplevel')


Potential worry if the script is run when CWD is outside of the rust repo (including possibly in any other repo)?

This script will create a Rust exercise in the exercises directory of whatever repo the CWD is currently in when the script is run. If the CWD is not in a git repo, the script breaks. If the CWD is in a repo without an exercises directory, the script breaks. No attempts are made to do anything smart in those cases; it just prints the traceback and exits.

In practice, I can't see anyone ever actually running this from within a non-Rust repo; I think the worry is more theoretical than practical. The bits which insert the entry into config.json are interactive anway, meaning that if someone runs this as a script they'll use the --dont-update-config flag, which means they can just cp or mv or rm the generated directory appropriately.

One might consider a solution that is not sensitive to where the CWD is: the script determines its own location, and acts on the config.json and exercises directories at the proper relative location to itself.

then we run into problems if the person running the script has moved it before running it.

I will say that I am far more likely to run this with CWD being outside of the repo than I am to move this script.

Yeah, I'm not too worried about people moving the script.

However, something like os.chdir(os.path.dirname(__file__)) before this line would probably fix both issues. I'll look into it.

Just pushed a version which does this; local testing confirms that the script now works correctly when CWD is outside the repo.

petertseng · 2017-11-08T03:38:26Z

bin/init_exercise.py

+    exercise_dir = os.path.join(EXERCISES, name)
+    # blank out the default lib.rs
+    with inside(exercise_dir):
+        with open(os.path.join('src', 'lib.rs'), 'w') as lib_rs:


truncate would also be acceptable.

petertseng · 2017-11-08T03:40:12Z

bin/init_exercise.py

+
+    cd = get_problem_specification(name)
+    if cd is None:
+        print("No problem specification for {} found".format(name))


~~someone running this might think "oh no! does that mean it failed and did nothing of use?!" so I would suggest to make it clear that it did in fact do something after not finding the specification.~~

~~someone running this might think "oh no! does that mean it failed and did nothing of use?!" so I would suggest to make it clear that it did in fact do something after not finding the specification.~~

I revoke my objection. It is handled, but in the respective functions (make_new_exercise prints "Creating new exercise from scratch")

petertseng · 2017-11-08T03:45:37Z

bin/init_exercise.py

+    else:
+        make_exercise_with_specifications(name, exercise_dir, cd)
+        with inside(exercise_dir):
+            generate_readme(name, True)


I had to look for definition of generate_readme to understand what the boolean argument is. I would suggest giving it a name.

same

petertseng · 2017-11-08T03:48:12Z

bin/init_exercise.py

+                print("Don't forget that `README.md` is automatically generated; update this within `.meta\description.md`.", file=description)
+            with open('metadata.yml', 'w') as metadata:
+                print("---", file=metadata)
+                print("blurb: \"{} (created {})\"".format(


Putting the "created" portion here may encourage contributors to leave it there.

I think we do not want to encourage that, because it doesn't seem to be relevant to http://exercism.io/languages/rust/exercises when the exercise was created. I would remove it.

petertseng · 2017-11-08T03:49:09Z

bin/init_exercise.py

+                print("source: \"\"", file=metadata)
+                print("source_url: \"\"", file=metadata)
+        with inside('tests'):
+            with open('{}.rs'.format(name), 'a') as tests_rs:


guess: a in case the runner of this script had already created some tests, and we do not want this script to overwrite them.

If this guess was correct, then there is no need to respond to it.

No; if the exercise directory already exists, make_exercise() aborts.

The 'a' is because make_exercise() pre-populates the file with use {name}::*;.

petertseng · 2017-11-08T03:53:38Z

bin/init_exercise.py

+
+        def write_cases(cases):
+            for item in cases:
+                if all(key in item for key in ('description', 'input', 'expected')):


Note that the input isn't necessarily in input, but it would be difficult to come up with something that works for all exercises. Some tracks have created ideas and you may find some of them in exercism/discussions#155 (comment) . I wouldn't mind enforcing in the problem-specifications schema that all inputs must appear under input (possibly as an object that contains multiple keys, since some problems have multiple inputs) but obviously that is far out of scope for this PR.

I'm aware that some specifications include their inputs under other keys, but I think the best way forward for scripts like this is to enforce that all input is in input (possibly as an object) and all expected results are in expected. As you say, that's an issue out of scope for this PR.

The closest I've seen to a schema for the canonical-data.json format is in the problem-specifications README, and that uses input and expected, so that's all I feel the need to support for now.

You're right that more often than not, input isn't actually used, which poses a problem. I may introduce code to do something like what vimscript does, now that you've pointed out the issue.

petertseng · 2017-11-08T03:59:21Z

bin/init_exercise.py

+    import argparse
+    parser = argparse.ArgumentParser(description='Create a Rust Track exercise for Exercism')
+    parser.add_argument('name', help='name of the exercise to create')
+    parser.add_argument('--dont-create-exercise', action='store_true',


I am used to seeing --create-exercise and --no-create-exercise, but I don't care and it might not be the case that this is an accepted convention.

I haven't seen any other tools of this type; this was all ab nihilo.

I feel that a verb in the argument is more expressive than simple negation.

- Call generate_readme(.. get_problem_specification) by keyword - Remove created time from blurb - For all cases without `input` specified, collect all non-specified fields into an `input` tuple - Use f-strings instead of .format() when possible - Add exercise-local .gitignore for Cargo.lock - Add option to use the maplit crate for exercises which use many maps in their tests. This can make the tests substantially more compact and readable. - Set Cargo.toml version according to specification version - Generate substantial inline documentation in the tests based on various descriptions and comments - Generate a distinct property test function for each property - Add hints about input tuples to property test functions - Add hints about likely implementations to property test functions - Ensure that the output compiles

coriolinus · 2017-11-08T13:12:01Z

Made some improvements. Turns out the pov exercise makes for a really good test case: it's not already in Rust, it has several sections with their own comments, and the cases involve deep nesting and internal comments and descriptions.

Ironically, it's one of the exercises which would probably require the most work to actually bring into the Rust track, even with this script doing much of the gruntwork, just because Rust makes you work harder to build recursive structures.

I should be more careful with using auto-prettying editors on files for which auto-prettying will generate large changesets.

Turns out that `-` is not a legal character in a module name, so we need to adjust the crate definition and the import in the test file

petertseng · 2017-11-08T19:30:18Z

bin/init_exercise.py

+            print("Cargo.lock  # We're building a library, not a binary", file=gitignore)
+            print("            # http://doc.crates.io/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries", file=gitignore)
+        with open(os.path.join('src', 'lib.rs'), 'w') as lib_rs:
+            lib_rs.truncate()


excuse me that this is the only comment I give right now instead of a full review, as my allotted time is up:

I meant https://docs.python.org/3/library/os.html#os.truncate such that the file no longer needs to be opened

But I'm not sure that actually makes a difference, so I actually don't care.

Huh, ok. It still works, so I suppose it's just a case where there are a few valid paths to accomplish a goal in the standard library.

This was a fun challenge to implement in Rust, but it also serves as a useful demo of the code generation implemented in exercism#389: past implementing the example, I had to edit five lines of code in tests/book-store.rs and three lines to add the stub in src/lib.rs. Everything else was generated automatically. Comments are invited pertaining both to this exercise and to the output of the exercise generator.

Turns out .gitignore doesn't know about line-trailing comments, so just move the comments to prefix the actual ignore line.

This script will always prettify config.json when it's run, as an inescapable part of its read-edit-write cycle. Therefore, I'm adding its edits here, so that users of the script aren't stuck with config.json edits which are irrelevant to their PR.

Also: - clarify .gitignore comment on Cargo.lock rules - fix bug in which Cargo.lock was printed to stdout

coriolinus · 2017-11-11T12:44:10Z

Went through the code one last time, and added a few bugfixes and a version bump. I think that once Travis signs off on this, it'll be ready to merge.

* Add exercise book-store This was a fun challenge to implement in Rust, but it also serves as a useful demo of the code generation implemented in #389: past implementing the example, I had to edit five lines of code in tests/book-store.rs and three lines to add the stub in src/lib.rs. Everything else was generated automatically. Comments are invited pertaining both to this exercise and to the output of the exercise generator. * Revert irrelevant config.json edits * Remove debug prints and generated comments not intended for the student

coriolinus added 3 commits November 7, 2017 17:28

Update README.md to include a note about the script

3ae2ddb

Improve generated readme.md for new exercises

146d386

petertseng reviewed Nov 8, 2017

View reviewed changes

coriolinus added 3 commits November 8, 2017 14:26

Remove extraneous README changes

125a3b4

I should be more careful with using auto-prettying editors on files for which auto-prettying will generate large changesets.

Adjust module names to legal values

c0863f5

Turns out that `-` is not a legal character in a module name, so we need to adjust the crate definition and the import in the test file

Bump version

709b482

petertseng reviewed Nov 8, 2017

View reviewed changes

coriolinus mentioned this pull request Nov 9, 2017

JSON Schema proposal: All inputs should appear under the input key exercism/problem-specifications#996

Closed

coriolinus mentioned this pull request Nov 9, 2017

Add exercise book-store #390

Merged

coriolinus added 7 commits November 9, 2017 10:00

Fix generated .gitignore

897b314

Turns out .gitignore doesn't know about line-trailing comments, so just move the comments to prefix the actual ignore line.

Make script's result independent of CWD at execution.

a39a34a

Also: - clarify .gitignore comment on Cargo.lock rules - fix bug in which Cargo.lock was printed to stdout

bugfix: update crate name in Cargo.toml even when no version specified

53cf843

bugfix: correct path to fetch-configlet

21df9ca

Write config.json with a newline at the end

d0dd849

Bump version to 1.0

ee9473e

coriolinus merged commit 65b9c8d into exercism:master Nov 11, 2017

coriolinus deleted the init-exercise-script branch November 11, 2017 13:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Init exercise script #389

Init exercise script #389

coriolinus commented Nov 7, 2017 •

edited

Loading

petertseng left a comment

petertseng Nov 8, 2017

petertseng Nov 8, 2017

coriolinus Nov 8, 2017

petertseng Nov 8, 2017

coriolinus Nov 8, 2017

petertseng Nov 10, 2017

coriolinus Nov 10, 2017

coriolinus Nov 10, 2017

petertseng Nov 8, 2017

petertseng Nov 8, 2017

petertseng Nov 8, 2017

petertseng Nov 8, 2017

petertseng Nov 8, 2017

petertseng Nov 8, 2017

coriolinus Nov 8, 2017

petertseng Nov 8, 2017 •

edited

Loading

coriolinus Nov 8, 2017

coriolinus Nov 8, 2017

petertseng Nov 8, 2017

coriolinus Nov 8, 2017

coriolinus commented Nov 8, 2017

petertseng Nov 8, 2017

coriolinus Nov 8, 2017

coriolinus commented Nov 11, 2017

		return sp.stdout.strip()


		REPO_ROOT = output_of('git rev-parse --show-toplevel')

Init exercise script #389

Init exercise script #389

Conversation

coriolinus commented Nov 7, 2017 • edited Loading

petertseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coriolinus commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coriolinus commented Nov 11, 2017

coriolinus commented Nov 7, 2017 •

edited

Loading

petertseng Nov 8, 2017 •

edited

Loading