Update the README with some documentation for the new justfile and other features that were implemented (clash showtest etc.).

Describe the bug
Stub generator for Rust can produce invalid code when a variable is used in an arithmetic expression describing the number of times to loop. This is because in the expression n+1 as usize the as usize cast only applies to 1 instead of n+1.

To Reproduce

$ printf "read N:int\nloop N+1 write N\n" | clash generate-stub rust --from-file -
use std::io;

macro_rules! parse_input {
    ($x:expr, $t:ident) => ($x.trim().parse::<$t>().unwrap())
}
fn main() {
    let mut input_line = String::new();
    io::stdin().read_line(&mut input_line).unwrap();
    let n = parse_input!(input_line, i32);
    for i in 0..n+1 as usize {
        println!("N");
    }
}

$ printf "read N:int\nloop N+1 write N\n" | clash generate-stub rust --from-file - | rustc --emit=metadata -
error[E0308]: mismatched types
  --> <anon>:10:19
   |
10 |     for i in 0..n+1 as usize {
   |                   ^^^^^^^^^^ expected `i32`, found `usize`

Expected behavior
We could add parens like (n+1) as usize. Another option would be n as usize + 1.

Turns out CodinGame handles edge cases different than us in their casing logic:

    #[test]
    fn test_weirdness() {
        let word = "ABC1ABc1aBC1AbC1abc1";

        // (works) CodinGame snake_casing for C++, Python, Perl
        assert_eq!(VariableNameOptions::convert_to_snake_case(word), "abc1abc_1a_bc1ab_c1abc_1");

        // (FAILS) CodinGame snake_casing for C:
        assert_eq!(VariableNameOptions::convert_to_snake_case(word), "abc_1abc_1a_bc_1ab_c_1abc_1");

        // (FAILS) CodinGame casing for Pascal, Bash, Dart, Clojure(??), F#(??),
        // Groovy(??), Lua, ObjC
        // note that CodinGame does not even capitalize the word, it just keeps the capital 'A'
        assert_eq!(VariableNameOptions::convert_to_pascal_case(word), "ABC1ABc1aBC1AbC1abc1");

        // (FAILS) CodinGame casing for C#, D, Go, Java, JS/TS, Kotlin, PHP, Scala,
        // Swift, VB.NET
        assert_eq!(VariableNameOptions::convert_to_camel_case(word), "abc1ABc1aBc1AbC1abc1");

        // (doesn't exist) CodinGame casing for Haskell, OCaml
        // abc1abc1abc1abc1abc1
    }

I'm not sure how we want to handle this. I don't think it's necessary to match CodinGame behavior exactly but currently our convert_to_pascal_case would produce "ABC1aBc1aBC1AbC1abc1" which matches neither my expectation for PascalCase nor CodinGame behavior.

The difference in snake_casing C we can probably just ignore, I don't see any valid reason to have different snake casing for C?

This seems to be somewhat standardized way to turn off colored output: http://no-color.org/

Once this is implemented the --no-color option can be removed.

I'm imagining something like clash testcase 1 to print the input for the first testcase and clash testcase --output 3 to print the output for the third test case but other ideas are also welcome.

Describe the bug
Template generator produces invalid code in languages that don't allow shadowing and use a loop variable that is named the same as one of the names in the stub generator. Currently this only affects C++, but as more languages get added some of them will likely have similar issues.

To Reproduce

$ echo 'loopline N i:int' | clash generate-stub cpp --from-file -
#include <iostream>
#include <string>
#include <vector>
#include <algorithm>

using namespace std;


int main() {
    for (int i = 0; i < n; i++) {
        int i;
        cin >> i; cin.ignore();
    }

    return 0;
}
$ echo 'loopline N i:int' | clash generate-stub cpp --from-file - | gcc -x c++ -fsyntax-only -
<stdin>: In function ‘int main()’:
<stdin>:10:25: error: ‘n’ was not declared in this scope
<stdin>:11:13: error: redeclaration of ‘int i’
<stdin>:10:14: note: ‘int i’ previously declared here

CodinGame itself suffers from the exact same issue so we don't have plans to fix this on our side either.

This is just an issue where I thought we could keep track of the things that need to be done for us to hit a stable release.

• Publish to crates.io
  • Decide on a license
  • Decide on a package name
  • Keep the domain logic out of main.rs so that the library can be used independently
• Verify behavior with integration tests
  • Provide some method to specify a custom clashes directory
  • Add fixture clashes made for testing
  • Add CI that runs all tests
• Document library functions and classes with doctests
• #16
• #15

CG has a useful way to give feedback upon printing uninitialized (is it?) memory, but locally there is no way of knowing, and the STDOUT message seems to contain no mistakes (since the the characters are not printed / are zero width).

Link of the contribution where this was tested.

C code used:

#include <stdio.h>

int main() {
    int N;
    scanf("%d", &N);
    
    for (int i = 0; i < N; i++) {
        char num[15 + 1];
        scanf(" %[^\n]", num);

        int ptr = 0;
        int minus = 0;
        while (ptr < 15 && num[ptr] == '0' || num[ptr] == '+' || num[ptr] == '-') {
            if (num[ptr] == '-') {
                minus += 1;
            }
            ptr++;
        }

        if (minus % 2) printf("-");
        for (int i = ptr; i < 15; i++) {
            // This ends up printing uninitialized memory ???
            printf("%c", num[i]);
            // fprintf(stderr, "%d\n", num[i]);
        }
        printf("\n");
    }

    return 0;
}

When considering things like #20 and various other features #60 we will require to maintain some sort of configuration state.

I made this issue to discuss the interface for it. Assuming that we store it in a toml file to be consistent with the rest of the repo, we will probably have simple key/value strings.

Personally I recommend a git-like syntax:

$ clash config
workspace: not set
editor: not set
$ clash config editor vim
Successfully set editor to vim.
$ clash config
workspace: not set
editor: vim

Describe the bug
Turns out that even though the gameloop in Codingame stubs is intended for interactive puzzles, some clashes such as this Hanoi Tower one use it.

To Reproduce

clash fetch 5119910629a6bc760199c643306ecabb14c31
clash next 5119910629a6bc760199c643306ecabb14c31
clash generate-stub rust

Expected behavior
At the very least the stub generator should not crash unexpectedly. I don't think it's necessary to make gameloop actually work; we could either simply exit with an error, or ignore it (and print a warning) and print the part of the stub that our stub generator can handle. Either way the Parser::parse method should probably return a Result instead of panicking.

A clash statement may have something like

<<Next [[N]] lines:>>

in which case our formatter currently fails.

This would ideally be a set of justfile commands similar to what we had prior to this commit:

It should ease the process of:

• opening the file to edit (with whatever editor).
• (optional) go to the next clash.
• generate the stub.
• run the code on autosave.

Not all the steps are necessary, but I found those to be the bare minimum when solving puzzles / clashes.

If the solution takes more than 5 seconds for a test case it should be timed out. The timeout should (eventually) be configurable with --timeout option.

While messing with time consider also adding measurement of how long the solutions take in general and print it as "X/Y tests passed (took Z seconds)" to give some rudimentary idea of solution performance.

This crate might be useful: https://docs.rs/wait-timeout/latest/wait_timeout/

Revising this issue instead of opening a new one, please ignore the Old part.

Revised

Allow for a configurable workspace folder. Started a discussion for the config interface on #61

Allow for saving solution with these goals:

1. Atomic: the solution should be runnable without having to fetch the original clash json
2. Human-readable: A user who remembers a clash solved in the past should be able to navigate the workspace and find it easily
3. Multiple: The workspace should be able to organize multiple solutions for a puzzle
4. Notable: A user should be able to save a solution with a particular in order to cleanly differentiate solutions of a single exercise

Have a command (temporarily solve) which

• creates workspace/{clash-slug}/{lang}_{name}/ (lets call this folder $soldir)
• creates clash gen {lang} > "$soldir/solution.{lang extension}"
• copies the current clash's json to $soldir/puzzle.json

Have a command (temporarily commit) which

• saves the current solution with a name the user provides
• copies the current solution into a new $soldir to allow quick iteration

Make runner commands context-sensitive:

• If you run clash run (or a new porcelain command #60) from inside a $soldir it should auto detect puzzle.json and run the tests against it
• This would make it more convenient for users with integrated terminals in their editors

Slugging clashes and caching JSON

I think the public handle is unreadable as a folder name, it would be better if we slugged the title of the clash.

The rationale behind copying the clash JSON is to make this feature independent of codingame.tools and codingame.com. As long you have this binary and $soldir, you can still solve the puzzle.

Folder structure

I am personally against making a subfolder for each language, I feel like if the language is relevant to the solution it can be part of the solution name (if we name solutions by default like rust_1, js_1, js_2, etc.).

Plus, this tool is flexible enough to allow for solutions that use multiple language (say javascript + webassembly text) solutions.

Old:

Keep every solution in its own folder inside a folder for the clash:

- solutions/
  - {CLASH_ID}/
    - {timestamp}_{custom_solution_name}/
      - .git/
      - sol.rs
      - language # => rust
    - {timestamp}/ # solution without name
      - .git/
      - sol.py
      - language # => python

then allow for browsing solutions sorted by the timestamp with

clash solutions [CLASH_ID]

which would present the user with a select or fzf with the list of solutions for the clash of provided id or current clash if there is no id.
Selecting a solution should open it in the default editor.

Scenarios when I think it would be appropriate to create a new solution

• When the clash is changed with clash next or some other command there should be a flag that tells clash run to create a new solution automatically
• With a clash save <name> command which should save a new solution no matter what and set that solution as the current one being tested
• With a clash solve command

Scenarios when I think it would be appropriate to git add and git commit inside a solution:

• When a solution is edited and then tested with clash run, there could be a commit with the PASS/FAIL test status

coctus fetch causes a segfault on x86_64-unknown-linux-musl build downloaded from Github releases.

Version info: coctus_v0.2.0_x86_64-unknown-linux-musl

Related discussion in rust-build.action repo:

• rust-build/rust-build.action#49
• rust-build/rust-build.action#79

From reading those threads it seems likely that this issue is caused by reqwest statically linking against OpenSSL. Possible fix would be to use rustls-tls feature for reqwest.

Some help texts and such still use the old name clash.

Describe the bug
If you get a SEGFAULT in C, the run command output message is inconsistent.

STDOUT and STDERR being UB is acceptable I guess, but it really should notify the SEGFAULT if it's possible.

Apparently it is also inconsistent in CG, so it may be hard to fix.

To Reproduce
Write this C code in a tmp.c file at the root:

#include <stdio.h>

int main() {
    printf("Before SEGFAULT\n");
    fprintf(stderr, "STDERR Before SEGFAULT\n");

    // This doesn't SEGFAULT in CG but it does for me... weird
    // int seg[10];
    // seg[11] = -1;

    // This segfaults in CG but doesnt backtrace the faulty line
    // int seg[10];
    // seg[1000] = -1;

    int *ptr = NULL;
    *ptr = 10;

    printf("After SEGFAULT\n");
    fprintf(stderr, "STDERR After SEGFAULT\n");

    return 0;
}

Load any clash, then run the command:
cargo run -- run --build-command "gcc -o tmp tmp.c" --command "./tmp"

Expected behavior

What happens

We could use something like the dissimilar crate and print nice colored diffs when the actual output doesn't match the expected output.

Each part of the app has a ton of implicit dependencies on other parts of the app. Testing any particular module is a nightmare of stubbing and misusing and implicit dependencies on the state of the file system.

I propose we build a testing system that exposes sample testing data with a convenient interface and fast implementation. Here are some ideas:

• A high level src/test_helper.rs module (only compiled with cfg(test) that includes functions for:
  • Fetching test clashes
  • Making custom process::Commands for the aforementioned test clashes
• A collection of clash JSON files in the repo
• A tests/common/cli_helper.rs module that includes functions to set up the file system environment in a certain way to run integration tests directly on the CLI

If the handle is not alphanumeric PublicHandle::new error will be rescued at line 212 by:

let handle = self.handle_from_args(args).or_else(|_| self.current_handle())?;

The intended behaviour should be a crash (instead of showing the last clash loaded by next as it does now), with the message that is already present in PublicHandle: "Invalid clash handle '{}' (valid handles only contain characters 0-9 and a-f)".

Add a clash gen command similar to https://github.com/kyuridenamida/atcoder-tools that generates the template for solving a clash in a particular language.

This could deal with #13 by adding the extra text to the file.

Currently there's no way to show the stub generator for a clash. Problem is that some clashes (example) are nearly impossible to solve without having access to information from the stub generator.

Unsure how to fix, maybe add a command or CLI option to specifically show the stub generator? I don't think always printing it is a good idea because clashes that have important information in stub generator are rare.

Add a clash delete command so we can get rid of all the clashes we might dislike. It will delete the clash from our current database, and probably even blacklist it, so if in the future something like a new clash fetch all function was implemented*, it will still not appear with clash next.

Another option is just to blacklist the clash, don't delete anything, and add some blacklist checks in the random generation of clash next.

*This also makes me think. The clash fetch all will be sending something like 2k+ requests... Maybe forget about that and just ship already some clash database with the repo? A new user may find bothersome to have to manually fetch a decent bunch of clashes, were he to give the repo a try.

When running clash run it would be nice to know the number of the failing test case especially if there are lots of tests. This would allow users to easily run only the failing testcase with clash run --testcases N.

Probably the best way to tackle the issue would be to add a field into the TestCase struct when parsing a clash (maybe serde has a way to do this?). The numbering of tests should start from 1 (similar to how it is on Codingame), so test 1 is the first test (index 0 in test case array) and test 2 is the first validator (index 1 in test case array). When done this way the numbering keeps working nicely even if you run tests in a weird order like clash run --testcases 6,4,2.

Current output:

PASS Test 1
PASS Validator 1
PASS Test 2
PASS Validator 2
... more output ...
11/12 tests passed

Desired output:

PASS #1 Test 1
PASS #2 Validator 1
PASS #3 Test 2
PASS #4 Validator 2
... more output ...
11/12 tests passed

re: @ellnix (from #71)

I suggest

• Moving App to lib
• Moving all the helper functions (functions that do not have a corresponding command) into App
• Removing the functions that have commands corresponding to them from App and just having them as regular functions in main.rs (or as part of a Cli type)
• Making the App constructor to take a single package_name parameter: let app = App::new("dev.CoCtus.coctus");

This would then separate the responsibility between calling the commands and the helper functions which we could use in tests. It would also prepare for #61 by allowing configuration to be stored in App and be used by users of the lib.

I've given this some thought and I think that it would be better to expose a simpler API that is completely agnostic of the coctus CLI tool and its data/config (that means keeping App private as its only purpose is to hold references to the configuration directories). Users of the library should have the choice of using their own config/data if they want to be independent of coctus the command line tool.

There are probably some methods in App that do "too much" though. Ideally they would follow this recipe (limiting I/O to the edges of the app):

1. Doing I/O (reading args, environment variables and files)
2. Passing the data forward to pure, easy-to-test functions (which we may want to expose in lib)
3. Doing I/O (printing the output / errors)

Originally posted by @Andriamanitra in #70 (comment)

As much as I personally hate reverse mode there should still be some way to play them specifically. I'm unsure what the CLI for this should look like, maybe clash show --reverse which skips printing the statement (and maybe prints a warning if the current clash is not marked as reverse)?

It would be great if there was a lookup table with language names and the build & run commands that the tool automatically searches for. Some ideas:

# changes the default language to C
clash language c

clash run 
# runs `sol.{language_file_ext}` with default language

clash run --language rust
clash run -l rust
# runs sol.rs with rust

Ideally there would be a config file in $XDG_CONFIG_HOME/clash for Linux and %Appdata%/whatever for Windows like:

# languages.yml
ruby:
  run: ruby sol.rb
c:
  build: gcc sol.c
  run: ./sol
rust:
  run: cargo run --release
  workspace: rust
  code_origin: sol.rs
  code_destination: src/main.rs

- workspaces/
  - rust/
    - src/
    - Cargo.toml

The workspace feature would allow for supporting non standard libraries in languages like Rust. The workflow would be:

• Edit sol.rs
• clash run
  • cp {config['rust']['code_origin']} CONFIG_FOLDER/workspaces/{config['rust']['workspace']}/{config['rust']['code_destination']},
    i. e. cp sol.rs CONFIG_FOLDER/workspaces/rust/src/main.rs
  • {config['rust']['run']},
    i. e. cargo run --release
  • Evaluate solution

This is following @daxida's point that this repository is less easily recommendable to users of codingame since they may not be aware of the tooling of the language they are using in codingame.

In addition, we could have a greater spread of users if we do not implicitly expect them to be comfortable with piping and chaining commands.

I propose that we take a "git" approach with two classes of commands

• Plumbing: low level commands that are intended to build larger commands
• Porcelain: higher level commands that bundle behaviors of plumbing commands together

Every command we have so far can be considered a plumbing command, in this issue I wanted to get opinions on adding porcelain commands like:

1. Fetching a puzzle, saving the stub to a file, and opening it in the editor in one command
2. Another run-like command that builds and compiles the solution based on the language and warns users when tooling is missing from their system
3. A watch command or option to the above command that watches for file changes without needing to pipe to anything