jeffreykegler / yahc

Yet Another Hoon Compiler
MIT License
0 stars 1 forks source link

Extra indentation of kingside jog bodies. #6

Closed jeffreykegler closed 5 years ago

jeffreykegler commented 5 years ago

This issue is intended to replace issue #4 -- here's a list of kingside bodies with extra indentation. I'll follow with discussion in another comment.

commented.wuthep.txt

jeffreykegler commented 5 years ago

The warnings in this list are all kingside job bodies with extra indentation. In each case, a preceding comment aligns with the jog body. Most are indented one extra stop, but one is indented 4 extra stops and another is indented an odd number of spaces.

While other choices may be possible, here are the main choices that I see for the lint warnings in this issue. Here "acceptable" means "does not receive a high severity lint warning". If you start to analyze in terms of (the as yet unimplemented) multiple lint severity levels, the possibilities multiply.

  1. Treat none of them as acceptable, and record them all as anomalies.
  2. Treat all kingside jog bodies indented by extra stops as acceptable, if aligned with an immediately preceding comment.
  3. Treat all kingside jog bodies with extra indentation as acceptable, if aligned with an immediately preceding comment.
  4. Treat all kingside jog bodies indented by one extra stop as acceptable, if aligned with an immediately preceding comment.
  5. Treat all kingside job bodies indented by one extra stop as acceptable, whether or not they align with an immediately preceding comment.
jeffreykegler commented 5 years ago

oops.txt

I'm preparing a list of the other kingside warnings, and discovered I missed one that belongs with this set. It follows the most common pattern -- indented one extra stop, with an aligned comment in the immediately preceding lint.

jeffreykegler commented 5 years ago

@ohAitch -- I think we want to add these to anomaly.suppressions. Is that right?

ohAitch commented 5 years ago

I think something like 3? Basically, don't treat comments as whitespace for the purpose of determining jog body joinedness, at least until column 57; if there's a comment, the jog body should be joined, and indented either aligned or ragged(as should the comment itself).

Are there many split jogs whose heads have inline comments?

jeffreykegler commented 5 years ago

@ohAitch There are 8 of these. In all cases, the rune line and all other lines before the kingside jog body have a comment exactly at the point where a joined jog body would be aligned. It's as if the comment(s) is/are a "stand-in" or marker indicating that joined-style indentation is intended. I think it makes sense for hoonlint to enforce this. Do you agree?

ohAitch commented 5 years ago

sorry, to clarify: counting these as joined jogs, are there many that currently pass as valid split jogs(outdented or indented by 1 stop), but the rule I described would make stop linting?

jeffreykegler commented 5 years ago

I frankly don't remember. You're talking about applying the "comment requirement" to all split jogs? Does that include queenside? Off the top of my head, most do not have per-column-57 comments, but my memory may not be accurate.

ohAitch commented 5 years ago

This is a request to implement "hoonlint should enforce this" and see how much false positives it causes, yeah :)

On Thursday, 14 February 2019, Jeffrey Kegler notifications@github.com wrote:

I frankly don't remember. You're talking about applying the "comment requirement" to all split jogs? Does that include queenside. Off the top of my head, most do not have per-column-57 comments, but my memory may not be accurate.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffreykegler/yahc/issues/6#issuecomment-463718677, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxXhjLZb38jFD8OdJVGycCBfR4Lwzipks5vNZ5mgaJpZM4aymgh .

jeffreykegler commented 5 years ago

My guess is that it will create a mass of failures, but if you'd like, sure, I can do ahead and find out. That's one reason I spent a lot of time of allowing multiple policies earlier this week -- so rather than hack the "one true policy", we can create "burner" policies, throw them out, and see what happens.

jeffreykegler commented 5 years ago

Here's a census, right now just for hoon.hoon of the 1-jogging hoons, by kingside/queenside split/joined and commented/uncommented. For this purpose, "commented" means the gap between the head and the body of the jog contains at least one comment that begins before column 57.

commented_gaps.txt

Commented gaps are unusual -- of the WUTHEP split queenside's, only 18 of 207 have comments. OTOH all 3 of the hoon.hoon split kingside's have comments, although the only split kingside CENTIS has no comment. I'll run this census for all of arvo/ and pass on the results.

jeffreykegler commented 5 years ago

@ohAitch Here's the same census, as described in the previous comment, this time for all of arvo/.

arvo.commented_gaps.txt

The picture is not much changed from the one for just hoon.hoon. Let me know what you want to do -- look for more specific patterns, have me upload a more detailed of the census or a subset of it, etc etc.

ohAitch commented 5 years ago

A report of particular split-commented examples would be interesting, if there's only 20 of them; but yeah that sounds like "commented split should be treated as split when outdented from the end of the jog head, and as joined when indented from the end of the jog head" or something similarly liberal.

jeffreykegler commented 5 years ago

Here are the indented 1-jogging jogs with commented splits. I'll follow up with the outdented ones. There are only a few and in all cases, the jog body aligns with a comment on every preceeding gap line. Indentation varies -- 6 of the 11 are indented by two steps, but 5 of the 11 are indented by other amounts.

indent.lint.txt

jeffreykegler commented 5 years ago

Here are the outdented 1-jogging jogs with commented splits. They turned out more interesting than I thought they would be. In 53 of the 64 cases, there is a comment on every preceding gap line with the same alignment as the jog body -- this is the same pattern we see in the indents. And in every one of these 53 cases, the outdent is by 1 stop.

Of the other 9 of the 64, most seem to be cases where what is intended as a right-side comment is started before column 57.

outdent.lint.txt

jeffreykegler commented 5 years ago

@ohAitch Many choices here. Among them (note that the cases here overlap):

1.) Accept a jog body alignment whenever gap comment indicate that it is intentional. It's considered an indication of intention if and only if every line of the gap before the one with jog body contains a comment with the same alignment as the jog body.

2.) Follow the same indication of intent, but only accept it in restricted cases: 1 stop for outdent and 2 stops for indent.

3.) Liberalize the indication of intent, so that any pre-column-57 comment on any gap line indicates that the jog body indentation should be accepted.

4.) Liberalize the indication of intent, but only for restricted cases of indentation.

5.) Accept the comment's indication of intent only for indents, not for outdents.

Etc Etc

ohAitch commented 5 years ago

Ah okay this is a lot more consistent that I had been worried.

ohAitch commented 5 years ago

As for more general indent vs outdent, probably if this rule does cause a "misindented joined jog" lint, the autofixer should move the comment(splitting the jog) if the body is outdented, and reindent the body if the body is already somewhat indented; this is a separate question from "is there a lint in the first place" however ^-^

jeffreykegler commented 5 years ago

I think I understand what you want. This question is to confirm that, and also to make sure the implications of the rules are understood. I'll give 3 examples, the first of which is always lint-acceptable under your rules (as I understand), and 2 others which may produce lint warnings.

[ ../hoons/arvo/gen/test.hoon 40:5 indent tallWuthep split indented by 4 40:9 ]
40!     $|  ::  the stack is already flopped for output?
41          ;:  weld
42            p.run
43            `tang`[[%leaf (weld name " CRASHED")] ~]
44          ==

JOG BODY IS ALWAYS ACCEPTABLE: There is a comment on the jog head line which aligns with the jog body on the next line, which makes this jog "pseudo-joined" and the comment is one stop after the jog head, so that the alignment is correct for the body of a joined jog.

[ ../hoons/arvo/gen/musk.hoon 317:7 indent tallWuthep split indented by 4 317:11 318:11 ]
317!       $&  ::  if fully blocked, produce self
318            ::
319            ?^  blocks.mask.bus  bus

JOG BODY IS NEVER ACCEPTABLE: While the jog body aligns, it is not on the line after the jog head. Instead it is two lines past the jog head, so does not count as "pseudo-joined". It is therefore a split jog with the body indented two stops more than the head. Even in a kingside jogging, the body of a split jog should be indented only one stop, so this is not acceptable and lint will warn.

2464>   ?-  -.qer
[ ../hoons/arvo/app/hall.hoon 2465:7 indent tallWuthep split outdented by 2 2466:5 ]
2465!       $client
2466      ::  changes to shared ui state apply.
2467      ?+  -.det  ~
2468        $glyph  `[%client det]
2469        $nick   `[%client det]
2470      ==
2471    ::

ACCEPTABLE ONLY FOR QUEENSIDE: There is no comment on the line with the jog head, so this is not a pseudo-joined jog. The jog body is outdented one stop from the jog head, which would be acceptable in a queenside split jog.

ohAitch commented 5 years ago

Nah, all three are ACCEPTABLE

The first as you described; the second because while the body is not on the immediately next line, it is aligned(as are all intervening comments) to the pseudo-joining comment

The third is in fact not a pseudo-joined jog, and thus is a split jog; it is however indented entirely correctly for a split jog(at least from what context I can tell)

An actually NOT ACCEPTABLE would look something like:

?- foo %bar ::a b

On Monday, 18 February 2019, Jeffrey Kegler notifications@github.com wrote:

I think I understand what you want. This question is to confirm that, and also to make sure the implications of the rules are understood. I'll give 3 examples, the first of which is lint-acceptable under your rules (as I understand), and 2 others which will produce lint warnings.

[ ../hoons/arvo/gen/test.hoon 40:5 indent tallWuthep split indented by 4 40:9 ] 40! $| :: the stack is already flopped for output? 41 ;: weld 42 p.run 43 tang[[%leaf (weld name " CRASHED")] ~] 44 ==

ACCEPTABLE: There is a comment on the jog head line which aligns with the jog body on the next line, which makes this jog "pseudo-joined" and the comment is one stop after the jog head, so that the alignment is correct for the body of a joined jog.

[ ../hoons/arvo/gen/musk.hoon 317:7 indent tallWuthep split indented by 4 317:11 318:11 ] 317! $& :: if fully blocked, produce self 318 :: 319 ?^ blocks.mask.bus bus

NOT ACCEPTABLE: While the jog body aligns, it is not on the line after the jog head. Instead it is two lines past the jog head, so does not count as "pseudo-joined". It is therefore a split jog with the body indented two stops more than the head. In a kingside split jog the body should be indented only one stop, so this is not acceptable and lint will warn.

2464> ?- -.qer [ ../hoons/arvo/app/hall.hoon 2465:7 indent tallWuthep split outdented by 2 2466:5 ] 2465! $client 2466 :: changes to shared ui state apply. 2467 ?+ -.det ~ 2468 $glyph [%client det] 2469 $nick[%client det] 2470 == 2471 ::

NOT ACCEPTABLE: There is no comment on the line with the jog head, so this is not a pseudo-joined jog.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffreykegler/yahc/issues/6#issuecomment-464877455, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxXhrcZ5NmgF7NrsUF-cXgIozCaUyQBks5vOxTogaJpZM4aymgh .

ohAitch commented 5 years ago

Ah, I see I missed an edit. I see you caught the queenside thing; for the second one, it is the horizontal position of the eventual body that's important, not the horizontal one.

In general, everything in indent.lint.hoon is fine, and the only two jog-problematic lines in outdent.lint.hoon are the two "jog-related abnormalities" in the last bullet point I had two posts ago; there are also some yet-unwritten-lint problematic lines mentioned earlier(the unwritten lint is "column-57 margin comment isn't exactly on column 57")

jeffreykegler commented 5 years ago

I'd been holding back on this suggestion, because it would complicate an already complex discussion, but recall we discussed overriding lint warnings with inline comments, as eslint allows. This is fine for most lint warnings, but with the whitespace, where the point is often to have the layout clarify the code, the clutter of inline lint directives can be very counter-productive.

Perhaps the arvo/ corpus already points our way out of this dilemma. We could treat a header comment on a preceding line as "turning off" any lint alignment warning if it aligns with the jog body. Eventually we could extend this principle to other syntaxt elements.

This is an elegant a solution to an ugly problem, and it has the very great advantage of being strongly suggested by current arvo/ practice. Do you like the idea?

jeffreykegler commented 5 years ago

Note that I turned off the green question flag and am about to turn it on. I'll try to use the green question flag in this way to distinguish when I am asking a question from when I am just adding material that does not require feedback.

jeffreykegler commented 5 years ago

@ohAitch I have renamed the "jogging" document whitespace.md, and expanded it with the definitions you gave for comments, and new material on running hoons.

jeffreykegler commented 5 years ago

I've implemented the suggestions in the above, and silenced all lint warnings. Next I'll document this. Once that is complete, I'll mark it yellow: "Ready to be closed".

jeffreykegler commented 5 years ago

@ohAitch Documentation added to the whitespace standards doc. I am marking this ticket as "About to be closed", and will close it shortly unless I hear to the contrary.

jeffreykegler commented 5 years ago

Closed as per the above.