[IMP] process disable per line instead per file in XML checks

[fernandahf]

This PR enabled the disabling of lints per XML file in checks, I think it would be great if we can disable per line in order to only disabling the lint in the line where you know it's a valid case.

[fernandahf]

FYI @antonag32, @moylop260

[antonag32]

Sounds like a very nice thing to have. I added the feature in a POC to refactor odoo-pre-commit-hooks:

https://github.com/vauxoo-dev/odoo-pre-commit-hooks/blob/14f5c71f8369ff9be81a502ec0aec694c273c13f/src/oca_pre_commit_hooks/linters/xml/base_xml_linter.py#L20

I think we need to define the syntax for it, so far I did it this way:

• Comments must be on the same line they want to disable
• Comments can't span multiple lines (this is because the lxml sourceline no longer matches the starting line when they span multiple lines AFAIK)
• The syntax itself is the same than the one used for file wide disabling

We can probably extend it so that comments directly above or on the same line as the tag disable all checks for the tag. However this brings another question:

Should checks be disabled for all elements inside the tag? Or just for the tag itself? I think disabling checks for all tags inside would be nice, since the logic for disabling checks for the whole file, and disabling checks per line would be the same so code can be reused.

For example:

<!-- oca-hooks:disable=.... -->
<odoo>
...
</odoo>

Is the current way of disabling checks for the whole file, but it can instead disable all checks for everything inside the tag which practically is the same.

This also means that if you have a view/element with lots of offending elements, you can disable checks with just one comment, instead of repeating the same comment multiple times.

What do you think? @fernandahf @moylop260 @luisg123v

[fernandahf]

@antonag32

I think If it's possible having the 2 options would be great:

• One option for disabling per tag
• One option for disabling a parent tag and therefore disabling the same check in the child tags.

@moylop260

What do you think?

[antonag32]

I think if we implement this behavior the choice is left to the user (they can choose any of the two options, internally the code would be the same --- this is the main benefit, besides more flexibility).

However I also think our check structure would need to change, otherwise we can enter recursive problems. For example, right now most XML checks use XPaths to find the desired element directly, if we let checks be disabled by parents we would need to look up the element's parent recursively (up to root) and make sure the check was not disabled anywhere else.

So instead of checks freely having access to the whole file, checks would probably need to receive certain xml elements (that have been previously validated not to disable the check in question), something like visit from python but for xml visit_xml_record or visit_xml_field.

Just an idea :thinking: