search

errata-ai / vale

:pencil: A markup-aware linter for prose built with speed and extensibility in mind.
https://vale.sh
MIT License
4.47k stars 154 forks source link

Ignore comments are not honored in a markdown or reStructuredText file inside a list #175

Closed gaurav-nelson closed 4 years ago

gaurav-nelson commented 4 years ago

I probably have an edge case. I have put in <!-- vale off --> comment (inside a numbered list) in my markdown file but its still reporting errors.

I have a numbered list with a YAML code example (test1.md) and have put it ignore comments for second list item. However, Vale is not honoring the ignore comments. But it works fine (show no errors) if I remove --- from my YAML example (test2.md).

I have added test1.md and test2.md below, and I am using Microsoft style.

Vale error

test.md

 44:8   error       Use "shouldn't" instead of      Microsoft.Contractions
                    "should not".
 44:29  suggestion  'be used' looks like passive    Microsoft.Passive
                    voice.
 45:4   suggestion  Verify your use of 'Ensure'     Microsoft.Vocab
                    with the A-Z word list.
 45:33  error       Use "aren't" instead of "are    Microsoft.Contractions
                    not".

✖ 2 errors, 0 warnings and 2 suggestions in 1 file.
test1.md

````` 1. Consider the following `deployment.yaml` file. ```yaml apiVersion: v1 kind: Service metadata: name: my-nginx-svc labels: app: nginx spec: type: LoadBalancer ports: - port: 80 selector: app: nginx --- apiVersion: apps/v1 kind: Deployment metadata: name: my-nginx labels: app: nginx spec: replicas: 3 selector: matchLabels: app: nginx template: metadata: labels: app: nginx spec: containers: - name: nginx image: nginx:1.7.9 ports: - containerPort: 80 ``` 1. few other steps. 1. The output lists the failure code and policy violation details. ```bash SSH should not typically be used within containers. Ensure that non-SSH services are not using port 22. ``` something else. `````

test2.md

````` 1. Consider the following `deployment.yaml` file. ```yaml apiVersion: v1 kind: Service metadata: name: my-nginx-svc labels: app: nginx spec: type: LoadBalancer ports: - port: 80 selector: app: nginx apiVersion: apps/v1 kind: Deployment metadata: name: my-nginx labels: app: nginx spec: replicas: 3 selector: matchLabels: app: nginx template: metadata: labels: app: nginx spec: containers: - name: nginx image: nginx:1.7.9 ports: - containerPort: 80 ``` 1. few other steps. 1. The output lists the failure code and policy violation details. ```bash SSH should not typically be used within containers. Ensure that non-SSH services are not using port 22. ``` something else. `````

jdkato commented 4 years ago

Thanks for the report.

Are you able to share the actual Markdown file (or a functionally equivalent copy of) containing the <!-- vale off --> comments? It's not clear to me where exactly these comments are from your rendered examples.

gaurav-nelson commented 4 years ago

Thanks @jdkato

Here are the contents of:

  1. test1.md file

1. Consider the following `deployment.yaml` file.
   ```yaml
   apiVersion: v1
   kind: Service
   metadata:
     name: my-nginx-svc
     labels:
       app: nginx
   spec:
     type: LoadBalancer
     ports:
     - port: 80
     selector:
       app: nginx
   ---
   apiVersion: apps/v1
   kind: Deployment
   metadata:
     name: my-nginx
     labels:
       app: nginx
   spec:
     replicas: 3
     selector:
       matchLabels:
         app: nginx
     template:
       metadata:
         labels:
           app: nginx
       spec:
         containers:
         - name: nginx
           image: nginx:1.7.9
           ports:
           - containerPort: 80
  1. few other steps.
  2. The output lists the failure code and policy violation details. 
    SSH should not typically be used within containers.
Ensure that non-SSH services are not using port 22.

something else.


2. `test2.md` file. (Same as `test1.md` without `---` appearing in yaml example.

  1. Consider the following deployment.yaml file.

    apiVersion: v1
kind: Service
metadata:
 name: my-nginx-svc
 labels:
   app: nginx
spec:
 type: LoadBalancer
 ports:
 - port: 80
 selector:
   app: nginx

apiVersion: apps/v1
kind: Deployment
metadata:
 name: my-nginx
 labels:
   app: nginx
spec:
 replicas: 3
 selector:
   matchLabels:
     app: nginx
 template:
   metadata:
     labels:
       app: nginx
   spec:
     containers:
     - name: nginx
       image: nginx:1.7.9
       ports:
       - containerPort: 80

  2. few other steps.

  3. The output lists the failure code and policy violation details.

    SSH should not typically be used within containers.
Ensure that non-SSH services are not using port 22.

something else.

amyq commented 4 years ago

I may be muddying the waters here, but I've seen similar behavior when attempting to use <!-- vale off --> on a specific list item in an unordered list. My team at the time found that we had to wrap the entire ordered list in <!-- vale off --> / <!-- vale on --> tags, even if we just wanted to disable linting on a single item in the list. @gaurav-nelson have you tested this behavior?

I had assumed at the time that this was a ReStructuredText issue, but I'm tackling https://gitlab.com/gitlab-org/gitlab/issues/208081 today in documentation that uses Markdown, and I'm finding the same result. Single list item == no disabling, but disabling linting on the entire list works properly.

gaurav-nelson commented 4 years ago

Thanks @amyq for chiming in. That seems a bit strange, I have noticed this behavior sometimes but wasn't able to identify the cause. In my not so real world test the following worked as usual, that is Vale didn't show errors and honored the off and on comments. 

## Heading One
This is an introductory paragraph to check if vale works inside an unordered
list.

- Unordered list example.
<!-- vale off -->
- SSH should not typically be used within containers. Ensure that non-SSH services are not using port 22.
<!-- vale on -->
- one more item.

something else.

@amyq Can you point to exact docs where you have had to ignore whole lists? I am guessing it has to do something with lists with code blocks or multiple level items.

esmerel commented 4 years ago

Since Amy can't get to that repo anymore, I've got a couple of examples. Hopefully I've pulled out the examples she was thinking of :)

In this file, you'll see vale off/on around an unordered list. The word 'Simple' is an error in our tests (I think it came out of write.good?) If I change it to put .. vale on in the middle of the list, it breaks the list (as I would expect it to in RST - /Users/lynette.miles/docs/docs/commerce/configuration.rst:76: WARNING: Bullet list ends without a blank line; unexpected unindent.)

This page is publicly available at: https://docs.acquia.com/commerce/configuration/

``.. include:: ../common/global.rst

Acquia Commerce Manager configuration settings

.. toctree:: :hidden: :glob:

/commerce/configuration/*

As an administrator, you need to configure the various sections of your Drupal website to ensure that it will synchronize your products, services, and users, while also ensuring that your website displays those items in the desired fashion. Understanding your configuration settings and where to find them will help you set up your commerce website properly. Here, we discuss the various commerce configuration settings:

You can access the settings for each of the previous items by completing the following steps:

. Sign in to your Drupal website as an administrator.

. Go to Commerce > Configuration.

. Click the specific configuration link.

. Make your changes.

. Click Save or Save configuration.

.. _commerce-checkout-settings:

Checkout settings

Checkout settings control the flow of the checkout process for your users. You can add custom checkout flows with development work.

.. _commerce-sku-settings:

SKU settings

SKU settings control the types of SKUs that your commerce website uses. These are added by your commerce solution and synchronized to Drupal. The default types are as follows:

.. vale off

.. vale on

.. _conductor-settings:

Commerce Connector Service settings

Commerce Connector Service settings are described in the Acquia Commerce Manager Drupal module installation, in the :ref:Commerce Connector Service settings <commerce-config-connector-service> section.

.. _commerce-currency-settings:

Currency settings

Use the following currency settings to determine how your currency will display for your products and cart:

.. _commerce-user-settings:

Commerce user settings

Various user settings

.. _commerce-synchronize-promotions:

Synchronize promotions

The Synchronize promotions button on this page manually synchronizes promotion information between your Drupal website and your commerce solution.

Synchronize

This page has two buttons that enable you to manually synchronize data between your Drupal website, and your commerce solution. Clicking either button will attempt to synchronize its respective data, in a queue.

File 2: This file is using vale on an ordered list. Vale wraps the entire list, because the word 'Just' is an error in our tests. However, if you try to surround only that line (#. Enter a trigger type named Clicks - Just Links.) with .. vale off/vale on, it acts as though you've intended to break the numbering. given that there's blank lines, it makes sense. We always assumed this was a limitation of RST. This page is publicly available at: https://docs.acquia.com/lift/profile-mgr/event/

``.. include:: ../../common/global.rst

Creating and managing events

To help you understand how your website visitors interact with your website, and to receive a personalized experience, your website sends your visitors' interactions (also known as captures) to Acquia Lift. Depending on your website's size and implementation, you must consider thousands of types of interactions.

Acquia Lift includes descriptive containers called events you can use to compartmentalize your captures by a common theme (such as shares or registrations) or conversion events (including clicks).

.. _lift-creating-new-events:

Creating new events

To create new descriptive events for the website visitor captures Acquia Lift receives from your website, complete the following steps:

. |lpm-signin|_, and then click the Configure tab.

. From the left menu, click the Events link.

. Click the Add new event link.

. In the Event Name field, enter a descriptive name for the event

you want to track.

. In the Event ID field, enter the machine name for the event.

. In the Customer Site list, click the website to which you

want the event to apply. Click Global to use the event across all your websites.

. The Event Type list describes how visitors interact with your website.

. Click Save to create the new event. The new custom event displays in

the form as shown in the following screenshot:

|Custom Event|

After you create a new event, you must send the capture request from the webpage using Google Tag Manager <https://marketingplatform.google.com/about/tag-manager/>__.

To send the capture request, you must locate an existing trigger in Google Tag Manager, and create a tag with custom HTML.

For more information about creating or using an existing trigger, and creating a tag using Google Tag Manager, see :ref:`Creating a custom event capture with added variables

`. .. _lift-example-creating-capture-event-google-tag-manager: Example: Creating a Capture event with Google Tag Manager --------------------------------------------------------- In the following example, you must track when a person submits a quote request on your website. Although not comprehensive, the acceptance criteria has two parts: - The user has clicked on a link from your Drupal website with the URL containing ``thank-you`` within it. - You can :doc:`segment ` those users in Acquia Lift. Requirements ~~~~~~~~~~~~ - The website has Acquia Lift :doc:`installed `. - The website has `Google Tag Manager `__ installed. - The **Button Click** has the ``Element URL`` variable associated in Google Tag Manager. - The website has a **Thank you** page after form submission where the URL displays as ``/thank-you``. Follow the preceding process to :ref:`create a new custom event ` in Acquia Lift. .. _lift-creating-trigger-gtm: Creating a trigger in Google Tag Manager ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If a subscriber trigger already exists in Google Tag Manager, you must use the subscriber trigger instead. If no trigger exists, create a new one using the following process: .. vale off #. Sign in to your `Google Tag Manager `__ account. #. Click **Triggers**. #. Click **New**. #. Name the trigger *Submit Request Quote*. #. Enter a trigger type named *Clicks - Just Links*. #. Configure **This trigger fires on** to **Some Link Clicks**. #. Under **Fire this trigger when an Event occurs and all of these conditions are true**, configure the variable to an ``auto-event`` variable using ``Element URL`` containing ``thank-you``. You must create the auto-event variable: - If ``Element URL`` does not exist, create a new ``auto-event`` variable with the following information: - **Variable Type**: Element URL - **Component Type**: Path The **Variable Configuration** form for the ``auto-event`` variable displays as shown in the following screenshot: |Variable Configuration| #. Save the *Submit Request Quote* trigger. The new trigger displays as shown in the following screenshot: |Submit Request Quote Trigger| .. Vale on .. _lift-creating-tag-gtm: Creating the tag in Google Tag Manager ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ #. Sign in to your `Google Tag Manager `__ account. #. Click **Tags**. #. Click **New**. #. Name the tag *Lift - Submit Request Quote*. #. Add a **Custom HTML Tag**. #. From the HTML tag editor, paste the following script: .. code-block:: text .. note:: The ``capture`` can use any method within the :doc:`Lift API `. #. Under **Triggering**, select the checkbox, and associate the **Submit Request Quote** trigger with the **Lift - Submit Request Quote** tag. #. Save the **Lift - Submit Request Quote** tag to display like the following screenshot: |Submit Request Quote Tag| .. _lift-testing-tag-gtm: Testing the tag in Google Tag Manager ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ #. Sign in to your `Google Tag Manager `__ account. #. Click **Preview**. #. Open your website where you will see the Google Tag Manager preview in the window. #. From the menu, open the ``/thank-you`` login link in a new tab, leaving your Google Tag Manager preview intact. #. The **Lift - Submit Request Quote** tag will have fired, and will display in the Google Tag Manager **Summary** section under **Tags Fired On This Page**. .. _lift-testing-lift-received-data: Testing Acquia Lift received the data ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ #. Before logging into Acquia Lift, grab the Acquia Lift tracking ID from your :doc:`browser cookies `. - In Chrome, you can **Inspect**, and then open **Application > Cookies**. - From **Cookies**, copy the ``tc_ptid`` for your website URL. #. |lpm-signin|_, and from the **People** search field, paste the cookie tracking ID from the preceding step. #. Click your user, and open the **Profile Details** screen. #. You will see the **Property Membership Status** with a value of **True** for your user. You have now created a **Custom Event** in Acquia Lift, and have effectively used the Acquia Lift :doc:`JavaScript API ` to capture event data. .. _lift-managing-events: Managing your events -------------------- To list and manage the events available for your use (including removing those you no longer want to use), complete the following steps: #. |lpm-signin|_, and then click the **Configure** tab. #. From the left menu, click the **Events** link. #. To view a filtered list of events, complete the following steps: a. Enter your search criteria in the **Find events** field. Leave the field empty to display all events. b. In the **Customer Site** list, click the website whose events you want to display. Click **Global** to list all your websites' events matching the search criteria in the **Find events** field. #. Click **Find**. To edit an existing event, click its **Event Name**, and then change its values as required. Ensure you click **Save** to save your changes. To delete an event, find the event you want to remove, and then click its **Delete** link. .. note:: Removing an event does not delete the captures associated with the event from Acquia Lift. .. _lift-viewing-reports-based-events: Viewing reports based on events ------------------------------- After you create a campaign, and Acquia Lift begins to receive captures from visitors, you can use the **Person details** page to display the number of those types of events Acquia Lift received for a particular visitor. To view information about all events captured by Acquia Lift: #. |lpm-signin|_, and then click the **Analyze** tab. #. From the left menu, click the **Analytics Explore** link. #. Go to **Explore**, and in the **Event** profiles field, click **Event Name**. .. _lift-displaying-filter-settings: Displaying filter settings ~~~~~~~~~~~~~~~~~~~~~~~~~~ To display filter settings, click **Filter** to the right of the field name. .. _lift-limiting-report-results: Limiting report results ~~~~~~~~~~~~~~~~~~~~~~~ To limit the report's results to those visitors who have one or more of a particular event in their history, complete the following steps for the **Event Name** field: #. In the list of filtering options for the field, click the filter you want to use—for example, click **Contains** to limit search results to visitors with events in their history containing your search term. #. In the empty field, enter the event name for which you want to search. Depending on what you enter, Acquia Lift will prompt you with website events you can click. #. If required, click the plus sign ( **+** ) to create more event name filters. #. After you have made all your filter changes, click **Run**. The report displays information about the event or events you have specified. .. |Custom Event| image:: https://cdn2.webdamdb.com/1280_MHYqpeMk9vL6.png?1552318022 .. |Variable Configuration| image:: https://cdn2.webdamdb.com/1280_kUuJIx0fSbY4.png?1552329749 .. |Submit Request Quote Trigger| image:: https://cdn2.webdamdb.com/1280_6N42yKgoYL51.png?1552330651 .. |Submit Request Quote Tag| image:: https://cdn2.webdamdb.com/1280_AI2UaMftQk71.png?1552332593 .. Next review date 20200311 ``
gaurav-nelson commented 4 years ago

Thanks @esmerel

Looks like an .rst example. Since, GitHub rendered most of the text, I can't see .. vale off and on comments. Maybe create a Gist and share.

esmerel commented 4 years ago

Yep, sorry, I wasn't thinking: Here: https://gist.github.com/esmerel/5347abdcd49208a9f041c555790a68ce - I've put each file in so you can see the on/off commands in the raw text.

jdkato commented 4 years ago

So, it looks like there are three issues being discussed here:

  1. The first issue was the presence of --- causing Vale to ignore comments. This was an upstream bug (https://github.com/yuin/goldmark/issues/111) and will be fixed in the next release.
  2. The second issue was using <!-- vale off --> on a specific unordered list item. I wasn't able to reproduce this on a test file, but I'll gladly look into it more if you (@amyq) have a example failing case.
  3. The third issue was using .. vale off within a reStructuredText list. This simply appears to unsupported syntax as rst2html gives an error (Bullet list ends without a blank line; unexpected unindent.).