Skip to content
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.

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

[quickfort] overhaul quickfort alias documentation #1724

Merged
merged 6 commits into from Dec 12, 2020

Conversation

myk002
Copy link
Member

@myk002 myk002 commented Dec 6, 2020

editorial review would be appreciated, though I'm sure the community will eventually find any errors too : p

  • move the alias syntax and usage docs from dfhack-config/quickfort/aliases.txt to a proper guide written in RST. Add examples and more details.
  • move the alias library docs from data/quickfort/aliases-common.txt to the new guide
  • reorder aliases in aliases-common to match the order in the docs
  • factor out the character used to enter the stockpile config screen so we can use the same aliases for stockpiles and hauling routes (use 's' for stockpiles and '{Enter}' for hauling routes)
  • reference the new guide in the quickfort user guide
  • do an editorial pass of the quickfort user guide
    • change name to "Quickfort Blueprint Guide", but only in the text, not the filename, so we don't change the URL
    • add quickfort-blueprint-guide as a label, in addition to the existing quickfort-user-guide
    • changed table-like lists to actual tables
    • changed "grid" tables into "simple" tables where possible
    • used ':kbd:' markers whenever we refer to a single character
    • turned Meta blueprints and Notes blueprints sections into subsections of a new "Other blueprint modes" section, in preparation for a few new modes coming in -r5.
    • removed out-of date caveat about bookcases, display furniture, and offering places not being supported

- move the alias syntax and usage docs from dfhack-config/quickfort/aliases.txt to a proper guide written in RST. Add examples and more details.
- move the alias library docs from data/quickfort/aliases-common.txt to the new guide
- reorder aliases in aliases-common to match the order in the docs
- factor out the character used to enter the stockpile config screen so we can use the same aliases for stockpiles and hauling routes (use 's' for stockpiles and '{Enter}' for hauling routes)
- reference the new guide in the quickfort user guide
- do an editorial pass of the quickfort user guide
  - change name to "Quickfort Blueprint Guide", but only in the text, not the filename, so we don't change the URL
  - add `quickfort-blueprint-guide` as a label, in addition to the existing `quickfort-user-guide`
  - changed table-like lists to actual tables
  - changed "grid" tables into "simple" tables where possible
  - used ':kbd:' markers whenever we refer to a single character
  - turned Meta blueprints and Notes blueprints sections into subsections of a new "Other blueprint modes" section, in preparation for a few new modes coming in -r5.
  - updated out-of date caveat about bookcases, display furniture, and offering places not being supported
@myk002 myk002 changed the title [quickfort] overhaul quickfort alias documentation [WIP] [quickfort] overhaul quickfort alias documentation Dec 6, 2020
@lethosor
Copy link
Member

lethosor commented Dec 6, 2020

Going to try reopening this to see if that kicks off a ReadTheDocs build

@lethosor lethosor closed this Dec 6, 2020
@lethosor lethosor reopened this Dec 6, 2020
@lethosor
Copy link
Member

lethosor commented Dec 6, 2020

ReadTheDocs seems to be successfully building PRs now - builds are listed at https://readthedocs.org/projects/dfhack/builds/, and you can find this particular one at https://dfhack--1724.org.readthedocs.build/en/1724/
I haven't figured out why it's not reporting build status back to GitHub yet, though - I suspect it needs to be authorized at the organization level, but RTD isn't making it particularly clear what needs to change.

Update: ReadTheDocs thought it was connected to GitHub, but GitHub disagreed - I was able to reconnect it to my GitHub account from the RTD side, and that seems to have worked (see the "docs/readthedocs.org:dfhack" check below).

@myk002
Copy link
Member Author

myk002 commented Dec 6, 2020

That's beautiful! details link takes you directly to the main index. I like : )

I found a bug in the script code while testing the changes in this PR, so DFHack/scripts has been updated. This PR is now ready for review.

@myk002 myk002 changed the title [WIP] [quickfort] overhaul quickfort alias documentation [quickfort] overhaul quickfort alias documentation Dec 6, 2020
@myk002 myk002 requested a review from lethosor December 9, 2020 17:42
Copy link
Member

@lethosor lethosor left a comment

Choose a reason for hiding this comment

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

I am behind on these, apparently. I figured the comments I have so far might be useful (I haven't gotten past the "sub-aliases" section yet) - I'm hoping to get through the whole thing by tomorrow. Definitely an improvement!

Since this is documentation for an existing feature, I would like to get it into r4 if possible (also delayed due to real life and such) but it is also possible to roll out docs updates later to the "stable" branch without making a new DFHack release (they'd just be left out of the offline copy that users get).

docs/guides/quickfort-alias-guide.rst Outdated Show resolved Hide resolved

The syntax for defining aliases is:

::
Copy link
Member

Choose a reason for hiding this comment

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

There is a shorthand you can use in cases like these (not mandatory, of course) - ending the line above with a double colon will render a single colon and begin a code block. https://raw.githubusercontent.com/DFHack/dfhack/develop/docs/Compile.rst has some examples of this.

There's also a more explicit form, .. code-block::, that you can use to set syntax highlighting on a per-block basis, but the blocks in this document all seem to be displaying reasonably with the default "DFHack" highlighting (which basically just styles lines starting with # differently).

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you for this tip! several hundred superfluous lines are now deleted.

for the .. code-block:: format, how do you set the language for the syntax highlighting? I believe the default is python, which isn't always appropriate

Copy link
Member

Choose a reason for hiding this comment

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

.. code-block:: language
I made a couple changes in 09f262a and a5f85e2
I think the default looks good here, but you can also change the default for all following code blocks with the .. highlight:: language directive (Compile.rst sets this to shell initially)

Comment on lines 171 to 172
Note that DF does not handle keys that have more than a single modifier, so
sequences like ``{Ctrl}{Alt}a`` will result in an error.
Copy link
Member

Choose a reason for hiding this comment

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

This isn't quite true - the interface_keys starting with CUSTOM_ are only defined for up to one modifier, but DF supports binding any interface_key to combinations with multiple modifiers (e.g. ctrl-alt-e gets saved as [SYM:6:e], where 6 == 4 | 2 == CTRL | ALT).

Perhaps "DF" should be replaced with "quickfort" here? In any case, the default interface.txt (well, mine, which has minimal modifications) only uses 1, 2, and 4 modifiers for SYM entries, so this is a relatively unimportant feature to support in my opinion.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the note. Thanks! FTR, quickfort code does support the general case of multiple modifiers. I just noticed that interface.txt didn't have any multiple-modifier keys when I tried to test the functionality. I forgot that custom definitions could be added.


::

{quantumstopfromeast name="Trash Dump"}
Copy link
Member

Choose a reason for hiding this comment

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

It wasn't initially clear to me from reading this section that the quantumstopfromeast alias would need to include {name} in its definition. Could help to provide an example (minimal) definition of this macro.

Also, what happens if the name parameter is omitted? In the quantum_enable example below, there is an existing alias of that name that serves to provide default behavior, but it's less clear to me what would happen in this case (I assume it would expand to an empty string?).

And finally, including underscores in the names of aliases here might make them a bit easier to read, but that's not hugely important (especially if you're trying to match aliases defined elsewhere).

Copy link
Member Author

Choose a reason for hiding this comment

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

good feedback. text updated. I have it as a future task for myself to allow empty alias definitions so the "optional sub-alias" case is handled more elegantly.

underscores (and hyphens) are relatively new to alias names. all the "original" aliases are pure alphanumeric (since they used the same convention as Python Quickfort). I could rename the existing ones, but I would risk breaking blueprints that use them. I don't think there are many blueprints that would break at this point (there hasn't been very much time yet for users to create blueprints that use the alias library), but I can't be sure.

- use :: shorthand where we can
- clarify sub-alias docs
- remove note about multiple modifier keys not being supported
@myk002
Copy link
Member Author

myk002 commented Dec 11, 2020

updates pushed

Copy link
Member

@lethosor lethosor left a comment

Choose a reason for hiding this comment

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

The rest of the new stuff looks good!

@lethosor lethosor added this to In progress in 0.47.04-r4 via automation Dec 12, 2020
@lethosor lethosor merged commit 9a6dcc3 into DFHack:develop Dec 12, 2020
0.47.04-r4 automation moved this from In progress to Done Dec 12, 2020
@myk002 myk002 deleted the myk_alias_docs branch December 12, 2020 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
0.47.04-r4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants