Page MenuHomeFreeBSD

Vale: Add rule to avoid redundant link
ClosedPublic

Authored by salvadore on Apr 14 2023, 8:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 7 2024, 6:27 PM
Unknown Object (File)
Jan 18 2024, 1:29 PM
Unknown Object (File)
Jan 18 2024, 1:29 PM
Unknown Object (File)
Jan 18 2024, 1:28 PM
Unknown Object (File)
Jan 18 2024, 1:27 PM
Unknown Object (File)
Jan 14 2024, 11:33 PM
Unknown Object (File)
Dec 22 2023, 11:42 PM
Unknown Object (File)
Aug 15 2023, 1:03 AM
Subscribers

Details

Summary

Suggest to empty square brackets in link: macros when the displayed
text coincides with the URL.

Diff Detail

Repository
R9 FreeBSD doc repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

salvadore created this revision.

@bofh: I would like to add a ] at the end of my regular expression, so that it matches all link: macros where the URL and the displayed text coincide exactly, but I was unable to find a way to do so. Do you know how should I do to match a litteral ] with vale? Thanks.

@bofh, I have also noticed my rule does not work for URLs ending with /. Any idea what is wrong?

@bofh: I would like to add a ] at the end of my regular expression, so that it matches all link: macros where the URL and the displayed text coincide exactly, but I was unable to find a way to do so. Do you know how should I do to match a litteral ] with vale? Thanks.

I am not too good in adoc so your partial description of the problem didn't make any sense to me at all. :/

Can you share two examples of adoc partials which are right and wrong?

I am trying to match all occurences of link:X[X] where X is the same URL. The rule I have proposed has two issues:

  • since I am unable to put the ] character in the rule, it also matches link:X[X, link:X[X/subdir], link:X[XY where Y is any string;

e.g. the rule would detect a match in link:https://www.FreeBSD.org[https://www.FreeBSD.org is the FreeBSD website], but I do not want it to do it;

  • if X ends with a / the rule does not work, for example it does not match link:https://www.FreeBSD.org/[https://www.FreeBSD.org/], but it does match link:https://www.FreeBSD.org[https://www.FreeBSD.org].

The second issue is more important: I do not think we would have many occurences of the first one, but I have already found many of the second in this quarter FreeBSD Foundation status report.

Let me know what happens if you replace the contents with this:

extends: existence
message: "Displayed text coincides with URL, leave the brackets empty: %s"
ignorecase: true
level: suggestion
scope: raw
raw:
  - 'link:(.*)\/?\[\1\/?\]'

I have checked with the following in an adoc:

link:https://www.FreeBSD.org[https://www.FreeBSD.org is the FreeBSD website]
link:https://www.FreeBSD.org/[https://www.FreeBSD.org/]
link:https://www.FreeBSD.org/[https://www.FreeBSD.org]
link:https://www.FreeBSD.org[https://www.FreeBSD.org/]
link:https://www.FreeBSD.org[https://www.FreeBSD.org]

Thanks @bofh, switching from tokens: to raw: works and adding \/? is a nice improvement.

As you can see, I have merged the rule I proposed with the one you suggested. The differences with what you suggested are the followings:

  • I kept the level warning which seems more logical to me, but if for some reason suggestion is preferable I can change it;
  • I kept ([^ ]*) instead of (.*), which is slightly more precise. Using (.*) would also match lines such as link:hello[hello world] [hello[hello world] ], which we do not want (I admit that I do not see any occasion where anyone would want to write such a line, but who knows...).

Thanks @bofh, switching from tokens: to raw: works and adding \/? is a nice improvement.

As you can see, I have merged the rule I proposed with the one you suggested. The differences with what you suggested are the followings:

Thanks.

  • I kept the level warning which seems more logical to me, but if for some reason suggestion is preferable I can change it;

When I initially committed vale I had the notion that I will get blanket approval from my mentors so that I can work on fixing the pages and adding the relevant keywords in the exception list related to FreeBSD. However that didn't happen. So unless we are fixing the current problems I am not keen on raising the level and adding more rules.

  • I kept ([^ ]*) instead of (.*), which is slightly more precise. Using (.*) would also match lines such as link:hello[hello world] [hello[hello world] ], which we do not want (I admit that I do not see any occasion where anyone would want to write such a line, but who knows...).
It works

I'll try to take a look to this tomorrow.
Also to the other review related to vale

  • I kept the level warning which seems more logical to me, but if for some reason suggestion is preferable I can change it;

When I initially committed vale I had the notion that I will get blanket approval from my mentors so that I can work on fixing the pages and adding the relevant keywords in the exception list related to FreeBSD. However that didn't happen. So unless we are fixing the current problems I am not keen on raising the level and adding more rules.

I have lowered the level to suggestion, at least for now. I sincerly hope a solution to these problems can be found: I need a linter to work efficiently on status report. If new vale rules are stopped by these issues, I will need to switch to plan B, i.e. creating new tools. But it would be a suboptimal solution.

  • I kept ([^ ]*) instead of (.*), which is slightly more precise. Using (.*) would also match lines such as link:hello[hello world] [hello[hello world] ], which we do not want (I admit that I do not see any occasion where anyone would want to write such a line, but who knows...).
It works

I think I have badly expressed myself. I meant I do not want the rule to match link:hello[hello world] [hello[hello world] ], such kind of line is fine (although unlikely to ever appear).

  • I kept the level warning which seems more logical to me, but if for some reason suggestion is preferable I can change it;

When I initially committed vale I had the notion that I will get blanket approval from my mentors so that I can work on fixing the pages and adding the relevant keywords in the exception list related to FreeBSD. However that didn't happen. So unless we are fixing the current problems I am not keen on raising the level and adding more rules.

I have lowered the level to suggestion, at least for now. I sincerly hope a solution to these problems can be found: I need a linter to work efficiently on status report. If new vale rules are stopped by these issues, I will need to switch to plan B, i.e. creating new tools. But it would be a suboptimal solution.

Currently if I run vale . I get:

✖ 2639 errors, 170206 warnings and 75938 suggestions in 3339 files.

I think if I can work for a day or two I can reduce this to less than 5000. However noone is stopping you from committing as I just mentioned what was in my long term plan. :D

Works for me, but please, about making the changes in the 3339 files, please contact first doceng@

This revision is now accepted and ready to land.Apr 18 2023, 6:24 PM

Works for me, but please, about making the changes in the 3339 files, please contact first doceng@

ahhhhh, sorry, the idea is to change:

link:https://www.FreeBSD.org[https://www.FreeBSD.org] to link:https://www.FreeBSD.org[]

Then go ahead hehehe, my fault.

@salvadore Make sure you are also disabling this rule for other translations in the .vale.ini file while committing.

@bofh: I think I have now disabled the rule for translations. Can you confirm that I did it properly please? Thanks.

This revision now requires review to proceed.Apr 18 2023, 8:19 PM
This revision is now accepted and ready to land.Apr 18 2023, 8:32 PM
grahamperrin added a subscriber: grahamperrin.

There is link redundancy in other contexts, so please change the name of this rule; make it more specific.

Thanks

This revision now requires changes to proceed.Apr 19 2023, 6:32 AM

Technically Reduntant Links define something else which is true. @grahamperrin what do you suggest? RedundantURLLinks??

In D39569#903020, @bofh wrote:

… what do you suggest? …

If I understand correctly, this rule is oriented to the current workflow for status reports, so maybe:

RedundantLinks-status.yml

– or:

RedundantURIs-status.yml

@salvadore your thoughts?


In parallel, I'll aim to learn more about vale in general.

In D39569#903020, @bofh wrote:

… what do you suggest? …

If I understand correctly, this rule is oriented to the current workflow for status reports, so maybe:

RedundantLinks-status.yml

– or:

RedundantURIs-status.yml

@salvadore your thoughts?


In parallel, I'll aim to learn more about vale in general.

No, for all the project

In D39569#903020, @bofh wrote:

… what do you suggest? …

If I understand correctly, this rule is oriented to the current workflow for status reports, so maybe:

I don't think so as we pretty much use link everywhere in our documentation so it's applicable everywhere in our doc repo rather than only status.

RedundantLinks-status.yml

– or:

RedundantURIs-status.yml

From my previous experiences I will avoid rule names containing -.

@salvadore your thoughts?


In parallel, I'll aim to learn more about vale in general.

If I understand correctly, this rule is oriented to the current workflow for status reports

No, for all the project

I confirm this is meant for all the project. Of course, since I work mainly on status reports I look at what is useful for them first, but whenever I see a rule that can apply to a larger domain I try to design it for it.

@salvadore your thoughts?

Here are a few suggestions for a better name:

  • RedundantLinkMacros;
  • OverabundantLinkMacros;
  • OverspecifiedLinkMacros;
  • UnnecessarlyPreciseLinkMacros;
  • RepetitiveLinkMacros;
  • UseDefaultURLInLinkMacros;
  • UnneededOptionalArgumentInLinkMacros.

Maybe I prefer the last one, but it is long... Maybe we can shorten it to UnneededOptArgInLink?

The link: macro prefix is not required when the target starts with a URL scheme like https:.

The prefix is commonplace in status reports, it should be a rarity elsewhere.

The link: macro prefix is not required when the target starts with a URL scheme like https:.

The prefix is commonplace in status reports, it should be a rarity elsewhere.

It's not required, but can be used.
This rule is gonna be used in all parts of the documentation.

The link: macro prefix is not required when the target starts with a URL scheme like https:.

The prefix is commonplace in status reports, it should be a rarity elsewhere.

It's not required, but can be used.
This rule is gonna be used in all parts of the documentation.

Actually, the FreeBSD Documentation Primer even recommends using link::

As the AsciiDoctor documentation describes, the link macro is not required when the target starts with a URL scheme like https. However, it is a good practice to do this anyway to ensure that AsciiDoctor renders the link correctly, especially in non-latin languages like Japanese.

https://docs.freebsd.org/en/books/fdp-primer/book/#asciidoctor-links-external

The link: macro prefix is not required when the target starts with a URL scheme like https:.

The prefix is commonplace in status reports, it should be a rarity elsewhere.

It's not required, but can be used.
This rule is gonna be used in all parts of the documentation.

Actually, the FreeBSD Documentation Primer even recommends using link::

As the AsciiDoctor documentation describes, the link macro is not required when the target starts with a URL scheme like https. However, it is a good practice to do this anyway to ensure that AsciiDoctor renders the link correctly, especially in non-latin languages like Japanese.

https://docs.freebsd.org/en/books/fdp-primer/book/#asciidoctor-links-external

Ah, yes, in some languages AsciiDoctor had problems with the link macro, maybe these problems are fixed but is a good idea to use the link macro

This is probably a conversation for elsewhere … I really want to change that part of the primer …

I have renamed the rule to SuperfluousOptArgInLinks. If anyone can suggest a better name, please do. Otherwise I think we should commit it with the name I have just applied and then, in the future, we can improve it when we have better ideas. E.g. some day the rule might evolve into something that catches superfluous optional arguments in any macro, not just link:, and thus we might want to rename it SuperfluousOptArg.

Macro shipit:

We can always rename the rules as it's just changing the file name and updating the .vale.ini file. It's not too big of a hassle.

In D39569#904486, @bofh wrote:

Macro shipit:

We can always rename the rules as it's just changing the file name and updating the .vale.ini file. It's not too big of a hassle.

hahahaha

This revision was not accepted when it landed; it landed in state Needs Review.Apr 25 2023, 10:43 AM
This revision was automatically updated to reflect the committed changes.