WIP: rebased and modified PR #179 #216
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change began with recognizing DeclareSynonym and cascaded to involve a number of related changes. First, in order for DeclareSynonym to be useful, there must be some way to specify the manual item type for the identifier being declared. Hence this change also implements a @ItemType command to allow that. (Adding @ItemType also achieves the main recommendaton in issue gap-packages#74.) In documenting these two changes, it seemed best to have their documentation in gap/Parser.gi next to their implementation, on the general principle that documentation should be as close to the relevant code as possible. Further, in an effort at dogfooding, it seemed best for that documentation to be in AutoDoc format. This turned into a change in which doc/Comments.xml became doc/Comments.autodoc and became much smaller, as most of the documentation moved to gap/Parser.gi and gap/Markdown.gi. Being able to use Autodoc to document most of Autodoc then entailed a few other changes. First, Markdown-like syntax with backticks for code expressions that would suppress the AutoDoc behavior of the quoted items was essential. Second, it was useful to add an @Index command for generating the index entries. And third, reassembling Chapter 2 of the manual from portions in several files necessitated allowing sections and subsections inside a chunk to move with that chunk, and be inserted into the chapter or section at the point at which the chunk was inserted. (An implementation note on this last change. It was greatly facilitated by merging the current_item ambient variable with the context_stack. The current_item was in essence functioning as the top of the stack anyway. Having the top of the stack in one variable and the rest of the stack in another made some of the code a bit confusing. This change touched a large fraction of code in the Parser, but in the end improved the locality of reference in that code considerably, and I think made it clearer what is going on during parsing.) In the course of these changes, it was trivial to add the ability to have code blocks which add at the current location, with no @insertcode necessary. So that change is included here as well. Finally, with so many changes, extensive testing was clearly critical. So this commit also beefs up the worksheet.tst, and adds a new test dogfood.tst which consists of generating the AutoDoc manual and verifying that it matches the expected contents of the manual. I do understand that this turned into more of a complex of changes than would ordinarily be packed into a single commit. But this portmanteau seemed a fairly natural stable point in which the motivating changes (DeclareSynonym/@ItemType) are made, documented, and tested in a cohesive way. I would be happy to do the work of separating this out into more than one successive change to AutoDoc if that would help advance these changes. Please just let me know what pieces you'd like it carved up into, and in what order. Thanks very much for your consideration. Resolves: gap-packages#60, gap-packages#74, gap-packages#174, gap-packages#175, gap-packages#176, gap-packages#177, gap-packages#178. It also makes further progress on gap-packages#48, in adding the backtick code quotes, and on the testing issues gap-packages#57 and gap-packages#152. It also touches on Issue gap-packages#112, in that to use makedoc.g in testing, I needed to add a backdoor to prevent it from QUITting. Note also that the motivation in the original post for Issue gap-packages#144 was the need to document DeclareSynonym.
|
I just wanted to apologize that the effort I put into PR #179 didn't receive follow-up. As I mentioned at the time, I would have been happy to divide it into a series of separate PRs. But I really did need guidance as to the order and the division of those. Also I received guidance not to use the "trailing @" convention but not any explanation of the alternatives. And sadly by the time there was a very nice clear message (in May 2019) outlining a potential path for those changes, my research had turned in a direction where I was not using GAP at all. Right now I am using very different tools, and so again, sadly I just can't justify returning to make a significant time effort on a tool I am not using. If my research does bring me back again to GAP, which is certainly possible, then seeing where AutoDoc stands and how I can help move it forward will certainly be important to me. Thanks all for your understanding of my situation, and thank you for all of the effort you put into AutoDoc, which was very helpful to me at the time. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is rebased and modified version of PR #179. I restore the
doc/Comments.xmlfile, and only kept thedoc/Comments.autodocfile in case it contains code edits we may want to borrow. I also removed all the inline autodoc comments added to the parser: While I agree that moving the comments there is good idea, these changes made it almost impossible to review the rest of the changes, so I stripped it. This probably lost some documentation edits, though.Anyway, this is not for merging, but rather I want to be able to view the code changes, and then possibly cherry-pick parts of this into master. The whole thing is untested, and certainly has issues, so do not merge this!
In principle, almost all changes in PR #179 sound good to me, but there is no way we are going to merge them in the form of one gigantic unreviewable blob.