Don't break lines in some blocks

[sadym-chromium]

To ease text search, added check verifying:

1. No break lines in [=...=] blocks.
2. No break lines in [...] blocks.
3. No break lines in |...| blocks.
4. No break lines in <var>...</var> blocks.
5. No break lines in <dfn>...</dfn> blocks.

And fixed existing issues in code.

---

Preview | Diff

[jgraham]

I'm inclined not to merge this kind of change unless we have some way to maintain the formatting going forward. Otherwise it's going to regress again pretty quickly.

[sadym-chromium]

I'm inclined not to merge this kind of change unless we have some way to maintain the formatting going forward. Otherwise it's going to regress again pretty quickly.

I don't know an automatic way to handle this, so I don't have a better idea rather then rely on authors and reviewers.

[sadym-chromium]

I added an agenda to today's meeting.

[sadym-chromium]

Not sure if we should keep <dfn>...</dfn> like in dfn>local end \n definition</dfn> in the same line though.

[sadym-chromium]

I misread your message as "inclined to merge", so continued working on that.

From one side, checking string length manually doesn't seem convinient. From the other, the overhead doesn't seem to be that big comparing to the advantage of being able to search without tricky regexps.

[juliandescottes]

This sounds easy to test with a small script, which could be triggered automatically via github actions. Don't know if we want to add a package.json here, but the following should do the trick?

const fs = require('fs');

const lines = fs.readFileSync('index.bs').toString().split("\n");
let failed;
for(let i = 0; i < lines.length; i++) {
    const line = lines[i];
    if (line.lastIndexOf("[=") > line.lastIndexOf("=]")) {
        console.log(`Unclosed [= ... =] at line ${i + 1}`)
        console.log(`L${i}: ${line}`);
        failed = true;
    }

    if ((line.split("|") - 1).length % 2) {
        console.log(`Unclosed | ... | at line ${i + 1}`)
        console.log(`L${i}: ${line}`);
        failed = true;
    }
}

if (failed) {
    throw new Error("Unclosed '[= ... =]' or '|...|'");
}

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Add the Element Origin concept.

The full IRC log of that discussion

<sadym> https://github.com//pull/208
<jgraham> github: https://github.com//pull/208
<jgraham> sadym: I wanted to find things in the spec, but found that line breaks interfered with that. I reformatted the spec, but there was pushback that it wasn't automatically enforced. jdescottes suggested a script to detect the issue, but I'm not sure how to set it up.
<jgraham> (for CI)
<jgraham> Scribe AlexRudenko1
<jgraham> q?
<jdescottes> q+
<jgraham> ack jdescottes
<whimboo> https://github.com/w3c/webdriver-bidi/blob/master/scripts/build.sh
<jgraham> q+
<AlexRudenko1> jdescottes: I proposed the script but I don't know what is the preferred tooling. Happy to try smth like this if there is nothing to lint the spec. I can see how to set up github actions to lint
<sadym> q+
<whimboo> q+
<jgraham> ack jgraham
<sadym> q-
<sadym> q+
<whimboo> https://github.com/w3c/webdriver-bidi/blob/master/scripts/test.sh
<whimboo> q-
<AlexRudenko1> jgraham (IRC): in terms of CI, we already have a job set up that is installing node and running the validation stuff. Because CDDL extraction is still running in node. I think the script will be easy to add to this job. The only problem is that the lint does not automatically fix it and increases the feedback time. If it is irritating enough we can probably write a script to fix it automatically
<jgraham> ack sadym
<AlexRudenko1> sadym (IRC): I can take another look at that PR and try to implement one of the options. I can take it over.

[whimboo]

I've updated the comment from the css meeting bot given that most of the details were related to other topics.

As we agreed on @sadym-chromium will have a look in how to get it integrated into the current scripts that we already run in CI - especially where we install node.

[sadym-chromium]

I added the checks and reformatted the code.

[sadym-chromium]

WDYT @jgraham @whimboo ?

[whimboo]

I think that this looks good and the formatter detects line breaks correctly. I wonder if we should also add this check to the test.sh script so that it is not always necessary to push the PR. This would save a lot of resources.

[sadym-chromium]

I think that this looks good and the formatter detects line breaks correctly. I wonder if we should also add this check to the test.sh script so that it is not always necessary to push the PR. This would save a lot of resources.

done

[whimboo]

Thanks Maksim! @jgraham given that you had concerns regarding missing verifications for upcoming changes can you please do the final review?

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Don't break lines in some blocks by sadym-chromium.

The full IRC log of that discussion

<jgraham> Topic: Don't break lines in some blocks by sadym-chromium
<jgraham> GitHub: https://github.com//pull/208
<jgraham> sadym: This PR adds a restriction to not add breaks in specific markdown constructs to make it easier to search in the markdown. We've added some tooling to help enforce this. This PR will involve lots of editing changes to the spec, so it would be nice to land it soon to avoid the need to rebase it and other PRs.
<jgraham> jgraham: I will try to look at that. We'll need to coordinate to avoid conflicts when landing

[jgraham]

I wonder if using https://github.com/domfarolino/specfmt is a better approach here (I haven't tested it yet).

[jgraham]

Apparently that doesn't try to enforce this kind of condition yet (and therefore this PR would make us incompatible with using that tool). Bleh. Would be really nice to have some proper autoformatting for specs that could enforce this kind of property.