Skip to content

Add test in scripts repo, to avoid manual documentation of keybinds #988

Closed

Description

@PeridexisErrant

This is automatically generated (see #972), which guarantees it will remain up-to-date. If a keybinding is suggested, it should simply be added to dfhack.init-example instead of the script documentation.

A test should be added to enforce this, probably by pattern-matching "keybinding add" or "bound to" and so on within the docs section.

Activity

  1. lethosor commented on Aug 31, 2016

    @lethosor
    Member

    So you're effectively proposing blacklisting those phrases from any script docs?

  2. dscorbett commented on Aug 31, 2016

    @dscorbett
    Contributor

    There’s bound to be false positives.

  3. PeridexisErrant commented on Sep 2, 2016

    @PeridexisErrant
    ContributorAuthor

    Yes, but due to the obvious false-positive issue it should probably be a very restricted blacklist. Part of the purpose is to have something in the scripts repo that tells people docs don't need to include keybindings, so just banning keybinding add commands should do - there's no other reason to use them I can think of, and if something comes up the test can always be modified later.

    As a regexp "keybinding add (Ctrl-|)(Shift-|)(Alt-|)[A-Z](@.? | )" within the script documentation tokens should work. I think, I'm not great with regex :/

  4. lethosor commented on Sep 2, 2016

    @lethosor
    Member

    Ctrl/Shift/Alt don't have to be in that order, so something like keybinding add (Ctrl-|Shift-|Alt-)*\w+(@.+)? should work. (Numbers, F1-F12, and Enter are supported too.)

    Anyway, I feel like having a person verifying this would be less error-prone than automatically failing a test. Maybe a warning would work, though (if anyone reads it :) ).

    Regarding #972 - that would add keybindings to the script docs as soon as they exist in dfhack.init-example, right? Just want to make sure it wouldn't require people to add anything to scripts they submit here, since that could cause issues if the corresponding dfhack.init-example changes haven't been merged (or thought of) yet.

  5. PeridexisErrant commented on Oct 20, 2016

    @PeridexisErrant
    ContributorAuthor

    Coming back to DFHack after a period of hibernation...

    The principle behind this is that people don't read warnings unless the build fails, and we can probably make this happen. We can tolerate false-negatives (hopefully manual review will catch them), so long as the false-positive rate is low.

    Yes, the idea of #972 is that scripts have nothing to do with it, and the note gets added automatically based on the contents of dfhack.init-example. Only plugins need to use the directive directly. In fact, our test should check that scripts aren't using it! (which would break when things change)

  6. lethosor commented on May 16, 2018

    @lethosor
    Member

    There were a couple unimplemented keybindings that I just fixed, but I think we can handle this manually in the future by deciding whether to add bindings to the default init file when PRs are made.

  7. added a commit that references this issue on Jun 27, 2018
    6b34a8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions