mbj4668 / pyang

An extensible YANG validator and converter in python
ISC License
530 stars 343 forks source link

YANG _emit_stmt() logic is wrong for multi-line arguments #775

Closed wlupton closed 2 years ago

wlupton commented 2 years ago

I believe that the YANG _emit_stmt() logic is wrong for multi-line arguments. The YANG shown below has two presence statements, both of which are 66 characters long but the first has a multi-line argument.

With this command (using the latest pyang from GitHub master):

% pyang presence-bug.yang --format yang --yang-line-length=66

the first one is (wrongly) wrapped to the next line, but the second one (correctly) isn't:

      presence
        "Presence of an aardvark strongly suggests that the
         'a' key is working correctly.";
      presence "Presence of a bison suggests that we should run.";

There appears to be an "off by three" error, because it's necessary to set --yang-line-length=69 to prevent the first one from wrapping:

      presence "Presence of an aardvark strongly suggests that the
                'a' key is working correctly.";

I think that I see what the problem is (actually I think there are two separate problems) and will create a PR.

YANG

module presence-bug {
  namespace "urn:presence-bug";
  prefix presence-bug;
  container root {
    container aardvark {
      description "The length of the next line is 66.";
      presence "Presence of an aardvark strongly suggests that the
                'a' key is working correctly.";
    }
    container bison {
      description "The length of the next line is 66.";
      presence "Presence of a bison suggests that we should run.";
    }
  }
}
fredgan commented 2 years ago

Wow, you are really careful!

wlupton commented 2 years ago

@fredgan, someone complained! We pass all BBF YANG through the edit transform to add metadata and reformat it, and people noticed that some of the presence statements were unnecessarily wrapped.

wlupton commented 2 years ago

Hmm. There doesn't seem to be a reference to PR #776. Now there is.