Closed jeffreykegler closed 5 years ago
The parse:829 children seem underindented if anything(one child then running is like consistent with some other things but might be a pain to formalize)
As for the rest, how many lines would following these corrections affect? I'm reading "these comments and cores are underindented" as "indent the majority of their containing file by a stop", which seems questionable: in ames we definitely want to /dedent/ the =~ and previous line, have not looked at the full context of the parse.hoon |%s
On Tuesday, 9 April 2019, Jeffrey Kegler notifications@github.com wrote:
These are the remaing TISSIG (=~) anomalies. All of these look like plain old mistakes to me and so should be added to anomaly.suppressions?
@ohAitch https://github.com/ohAitch Yes? Or do have another thought? The relevant lint output follows.
tissig-misc.lint.txt https://github.com/jeffreykegler/yahc/files/3061751/tissig-misc.lint.txt
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffreykegler/yahc/issues/28, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxXhs24xkFBexowUkwu94Lh6VZ5S-5zks5vfU_JgaJpZM4cl_2D .
Re down-jet/parse.hoon:829:
829> =~ [ruc=ruc sep(bun bun)]
[ hoons/arvo/lib/down-jet/parse.hoon 830:7 Test::Whitespace tallTissig 0-running runstep #2 @830:7; overindented by 2 ]
830! (lazy:eat ruc)
[ hoons/arvo/lib/down-jet/parse.hoon 831:7 Test::Whitespace tallTissig 0-running runstep #3 @831:7; overindented by 2 ]
831! news:eat
[ hoons/arvo/lib/down-jet/parse.hoon 832:7 Test::Whitespace tallTissig 0-running runstep #4 @832:7; overindented by 2 ]
832! node:eat
[ hoons/arvo/lib/down-jet/parse.hoon 833:7 Test::Whitespace tallTissig 0-running runstep #5 @833:7; overindented by 2 ]
833! knot:eat
834 :: ~? bug seated+[nod blos] .
835 ==
Runnings allow the first line to contain any number of run elements, separarted by a stop. That is the case in line 829. After the first line, elements must be aligned with the rune (unless re-anchored). That is why the warnings.
Re sys/vane/ames.hoon:
4> => =~
[ hoons/arvo/sys/vane/ames.hoon 6:1 Test::Whitespace tallTissig 0-running runstep #1 @5:1; comment underindented by 2 ]
5! :: structures
[ hoons/arvo/sys/vane/ames.hoon 6:1 Test::Whitespace tallTissig 0-running runstep #1 @6:1; underindented by 2 ]
6! =, ames
7 =+ protocol-version=2
8 |%
I'm not sure I followed you, but complaints are issued for every line considered misaligned -- so the number of lines affected, and which need to be changed, is the number of lines complained about. One caveat: I'm not taking a "holistic" view of the file, just a per-hoon one, so fixing an indent for a parent hoon might create an new indentation which needs to be propagated.
In the ames case, the two lines complained about are expected to be aligned with the =~
rune.
@ohAitch: With the above two (I hope) clarifications, bouncing it back to you.
Right: the explicit complaint is on the =,
, which implicitly cascades down the =+
and |%
, each contained ++
, and their respective bodies. This is where I'm getting the "most of the file" figure; and I contest that "reindent most of %ames" is not a reasonable action for a hypothetical hoonfmt
to take, or at least that this should be seen as a sign requiring manual intervention.
Most of %ames is not misindented, top-level cores should for the most part be flush with the first column, it is desirable for the =~
rules to act in line with this. (In the case of ames, I believe the solution is to outdent the =~ and all parents instead, but the parse.hoon
=~ |% lint is similarly requesting everything be indented over by a stop)
Re: runnings being aligned with the parent rune, =~
runnings, or runnings in general? My memory is "only the parent rune and == are on the same column: the children are indented more, except in the case of =~ where the children may instead be indented a stop less" - but usage is the final arbiter. (Would it be straightforward to generate links to like, 5 non-anomolous runnings of various runes?)
Yes, TISSIG is special in that the runsteps align with the rune. I need to note this in the whitespace document.
Initially, I think we have to be resigned to missing a few forests while we are counting trees, so to speak. That's why the TASKS doc places hoonlint
before hoonfmt
, and we also have a manual per-file review phase before subimitting By one theory, our warning-by-warning approach will catch all errors, but I don't believe that theory -- looking at the whole file will show us things we've missed.
Similarly, when we submit hoonlint
-based PRs, our audience will be less fussy (is my guess) than if we rewrite their files with hoonfmt
.
So for now, I asking that we look at the indentation issues on a per-rune (plus re-anchoring) basis, even that means our warnings tell the user to indent things to where they ultimately do not belong.
Yeah, ultimately my objection does boil down to "the |% should not be indented by one, and stay where it is" :P
So, OK to add these to anomaly.suppressions
, with the understanding there are likely to be issues, per-file, and other, with the error messages, which will need to be dealt with when we do our per-file review? Or is something unresolved that I missed?
Added the suppressions in commit d9abf7a . Will mark this "ready to be closed".
Closed as per the above.
These are the remaing TISSIG (
=~
) anomalies. All of these look like plain old mistakes to me and so should be added toanomaly.suppressions
?@ohAitch Yes? Or do have another thought? The relevant lint output follows.
tissig-misc.lint.txt