z390development / z390

z390 Portable Mainframe Assembler and Emulator Project
GNU General Public License v2.0
40 stars 15 forks source link

Improve readability of options overview documentation #566

Closed abekornelis closed 2 months ago

abekornelis commented 2 months ago

Fixes Issue #562 Improve readability of the source for the options documenation

jyganci commented 2 months ago

I've made a pass through the file. Comments follow. The github view is confusing. Hope I've looked at the file correctly.

================================================================================ Begin

CODEPAGE() Your col 1 has ascii and ebcdic reversed -- should be ebcdic+ascii. Also, +LIST is optional, so should be in square brackets.

Your col 3 has YES as the default. Should be ISO8859-1+IBM1047 or CODEPAGE(ISO8859-1+IBM1047).

col 1 CODEPAGE(ebc+asc[+LIST]) col 2 MALE col 3 CODEPAGE(ISO8859-1+IBM1047) or ISO8859-1+IBM1047; pick one; I'd pick the second. col 4 Values are ebcdicCodePage+asciiCodePage[+LIST])."If +LIST ... match z/OS." is okay.

================================================================================ col 1 MAXCALL(50) -- MAXWIDTH(800) . Original had nr in parentheses. col 1 MEM(1) -- MNOTE(0) It is my opinion that nr is better. Even better -- num.

If you decide to leave the explicit values, which appear to be the corresponding default values,

  1. the value for MAXGBL, 1000000, is inconsistent with its default, 100000.
  2. the value for MAXPARM, 100000, is inconsistent with its default, 10000.

================================================================================ Col 4 MNOTE(0). Two things.

  1. "0 - default generates ..." should be "9 0 generate ...".
  2. "2 - generates MNOTE ... and suppresses ..." should be "2 - generate MNOTE ... and suppress ..."

================================================================================ Col 4 RMODE24. "... the 24-bit address line." should be "... the 16MB line."

Col 4 RMODE31. Two things.

  1. "... above the 31 bit address line." should be "... above the 16MB line." There are 2 occurrences in the paragraph.
  2. "... option MEM be set ..." shoud be "... option MEM to be set ...".

    End

jyganci commented 2 months ago

It appears that you updated the PR while I was reviewing it. Now I will have to start over.

I will wait to hear from you that you are done with updates before I start over.

abekornelis commented 2 months ago

It appears that you updated the PR while I was reviewing it. Now I will have to start over.

I will wait to hear from you that you are done with updates before I start over.

Hi John, I understand your remark - fortunately that change indicator is a false positive. I did merge the changes from the merge you completed. So that changed my branch. But that touched only the tz390.java source file. Since the PR itself remained unchanged, I'd never expected github would mark the PR as 'changed'. You can rest assured - despite github's mark, not a single bit was changed in the file you were reviewing. You can safely continue where you left off. I'm sorry for the confusion - I'd never expected this side effect.

jyganci commented 2 months ago

Looks like you reverted your changes???

ALLOW col 4. "... syntax including:" -> "syntax."

ASSIST col 4. Hyperlink doesn't show. Make sure it works.

RMODE24 col 4. "24 bit address line" -> "16MB line"

RMODE31 col 4. "31 bit address line" -> "16MB line". Two occurrences.

TIME(nr) col 1 and col 4. Col 1 should be "TIME or TIME(nr)" as iimplied in col 4. Confusing. Looks like 3 possibilities:

  1. TIME takes default of 15 seconds
  2. NOTIME no timing
  3. TIME(nr) sets time limit t nr seconds

TRACE col 1 and col 4. You removed "or TRACE(AEGILMPQTV)". Col 4 has ".. the following trace options can be set ...". Need to add sentence "See table below for details." somewhere in this paragraph.