[pve-devel] [PATCH docs v2 0/2] pve fix help buttons by accounting for keys in widget toolkit

[pve-devel] [PATCH docs v2 0/2] pve fix help buttons by accounting for keys in widget toolkit

[Stefan Sterz]

this patch series fixes an issue where `onlineHelp` keys, that were
only used by components in the widget toolkit, were not properly
mapped to the corresponding anchors in the docs. this lead to
affected help buttons only showing an error message.

this series first fixes `pve-docs` by adding keys that were already
used by the widget toolkit's components. it also makes `asciidoc-pve`
a bit less aggressiv in order to avoid keys that are set by cbind.
then it adds the toolkit as a build dependency to `pve-manager` and
scans the toolkit for keys there when building the front-end.


changes from v1 @ Thomas Lamprecht:

* scan the widget toolkit too and not just components in
  `pve-manager`


Stefan Sterz (2):
  pveum: add the "user_mgmt" reference to the documentation
  asciidoc-pve: ignore anchor names in curly braces

 asciidoc-pve.in | 2 +-
 pveum.adoc      | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Stefan Sterz (1):
  fix #4328: ui: add widget toolkit to dependencies of OnlineHelpInfo.js

 debian/control        | 1 +
 www/manager6/Makefile | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH docs v2 1/2] pveum: add the "user_mgmt" reference to the documentation

[Stefan Sterz]

since this key was missing from the PVE documentation, the TFA ui's
help buttons didn't work as they relied on it.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 pveum.adoc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pveum.adoc b/pveum.adoc
index cbd553a..52de14d 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -1,4 +1,7 @@
 [[chapter_user_management]]
+
+[[user_mgmt]]
+
 ifdef::manvolnum[]
 pveum(1)
 ========
-- 
2.30.2

[pve-devel] applied: [PATCH docs v2 1/2] pveum: add the "user_mgmt" reference to the documentation

[Thomas Lamprecht]

Am 11/11/2022 um 16:05 schrieb Stefan Sterz:
> since this key was missing from the PVE documentation, the TFA ui's
> help buttons didn't work as they relied on it.
> 
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>  pveum.adoc | 3 +++
>  1 file changed, 3 insertions(+)
> 
>

applied, thanks!

btw. for my reply on patch 2 I can just amend the commit message myself,
so no need for a v3; just confirm if I indeed understood correctly.

[pve-devel] [PATCH docs v2 2/2] asciidoc-pve: ignore anchor names in curly braces

[Stefan Sterz]

previously the scanner would detect some `onlineHelp` keys that are
set via CBind as anchor names. this would cause it to fail, as they
cannot be present anywhere in the documentation. no valid anchor name
can be wrapped in curly braces, as they need to be valid xml names.
hence it should be safe to just ignore all keys wrapped in curly
braces.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 asciidoc-pve.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/asciidoc-pve.in b/asciidoc-pve.in
index d638a38..c536371 100644
--- a/asciidoc-pve.in
+++ b/asciidoc-pve.in
@@ -465,7 +465,7 @@ sub scan_extjs_file {
     debug("scan-extjs $filename");
 
     while(defined(my $line = <$fh>)) {
-	if ($line =~ m/\s+onlineHelp:\s*[\'\"](.*?)[\'\"]/) {
+	if ($line =~ m/\s+onlineHelp:\s*[\'\"]([^{].*?[^}])[\'\"]/) {
 	    my $blockid = $1;
 	    my $link = $fileinfo->{blockid_target}->{default}->{$blockid};
 	    die "undefined blockid '$blockid' ($filename, line $.)\n"
-- 
2.30.2

Re: [pve-devel] [PATCH docs v2 2/2] asciidoc-pve: ignore anchor names in curly braces

[Thomas Lamprecht]

Am 11/11/2022 um 16:05 schrieb Stefan Sterz:
> previously the scanner would detect some `onlineHelp` keys that are
> set via CBind as anchor names. this would cause it to fail, as they
> cannot be present anywhere in the documentation. no valid anchor name
> can be wrapped in curly braces, as they need to be valid xml names.
> hence it should be safe to just ignore all keys wrapped in curly
> braces.
> 
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>  asciidoc-pve.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/asciidoc-pve.in b/asciidoc-pve.in
> index d638a38..c536371 100644
> --- a/asciidoc-pve.in
> +++ b/asciidoc-pve.in
> @@ -465,7 +465,7 @@ sub scan_extjs_file {
>      debug("scan-extjs $filename");
>  
>      while(defined(my $line = <$fh>)) {
> -	if ($line =~ m/\s+onlineHelp:\s*[\'\"](.*?)[\'\"]/) {
> +	if ($line =~ m/\s+onlineHelp:\s*[\'\"]([^{].*?[^}])[\'\"]/) {

IIUC this indirectly raised the minimum length of references to two characters,
not a deal breaker IMO as I don't really expect two characters to be used anytime
soon (maybe with unicode 🤔🧠💭 x)), but maybe hint it in the commit message.

>  	    my $blockid = $1;
>  	    my $link = $fileinfo->{blockid_target}->{default}->{$blockid};
>  	    die "undefined blockid '$blockid' ($filename, line $.)\n"

Re: [pve-devel] [PATCH docs v2 2/2] asciidoc-pve: ignore anchor names in curly braces

[Stefan Sterz]

On 11/11/22 19:22, Thomas Lamprecht wrote:
> Am 11/11/2022 um 16:05 schrieb Stefan Sterz:
>> previously the scanner would detect some `onlineHelp` keys that are
>> set via CBind as anchor names. this would cause it to fail, as they
>> cannot be present anywhere in the documentation. no valid anchor name
>> can be wrapped in curly braces, as they need to be valid xml names.
>> hence it should be safe to just ignore all keys wrapped in curly
>> braces.
>>
>> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
>> ---
>>  asciidoc-pve.in | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/asciidoc-pve.in b/asciidoc-pve.in
>> index d638a38..c536371 100644
>> --- a/asciidoc-pve.in
>> +++ b/asciidoc-pve.in
>> @@ -465,7 +465,7 @@ sub scan_extjs_file {
>>      debug("scan-extjs $filename");
>>  
>>      while(defined(my $line = <$fh>)) {
>> -	if ($line =~ m/\s+onlineHelp:\s*[\'\"](.*?)[\'\"]/) {
>> +	if ($line =~ m/\s+onlineHelp:\s*[\'\"]([^{].*?[^}])[\'\"]/) {
> 
> IIUC this indirectly raised the minimum length of references to two characters,
> not a deal breaker IMO as I don't really expect two characters to be used anytime
> soon (maybe with unicode 🤔🧠💭 x)), but maybe hint it in the commit message.
> 

yes that is correct. just to have made the suggestion: the "[^}]" could
be dropped here. that should get us back down to one character, but will
also filter the necessary keys. and anchors like "{key" are just as
invalid as "{key}" in asciidoc.

>>  	    my $blockid = $1;
>>  	    my $link = $fileinfo->{blockid_target}->{default}->{$blockid};
>>  	    die "undefined blockid '$blockid' ($filename, line $.)\n"
> 

Re: [pve-devel] [PATCH docs v2 2/2] asciidoc-pve: ignore anchor names in curly braces

[Thomas Lamprecht]

Am 14/11/2022 um 09:26 schrieb Stefan Sterz:
>> IIUC this indirectly raised the minimum length of references to two characters,
>> not a deal breaker IMO as I don't really expect two characters to be used anytime
>> soon (maybe with unicode 🤔🧠💭 x)), but maybe hint it in the commit message.
>>
> yes that is correct. just to have made the suggestion: the "[^}]" could
> be dropped here. that should get us back down to one character, but will
> also filter the necessary keys. and anchors like "{key" are just as
> invalid as "{key}" in asciidoc.
> 


or move it to a single one replacing the whole non-greedy part, something like
(untested): [^{}\[\]"']+

Re: [pve-devel] [PATCH docs v2 2/2] asciidoc-pve: ignore anchor names in curly braces

[Stefan Sterz]

On 11/14/22 11:07, Thomas Lamprecht wrote:
> Am 14/11/2022 um 09:26 schrieb Stefan Sterz:
>>> IIUC this indirectly raised the minimum length of references to two characters,
>>> not a deal breaker IMO as I don't really expect two characters to be used anytime
>>> soon (maybe with unicode 🤔🧠💭 x)), but maybe hint it in the commit message.
>>>
>> yes that is correct. just to have made the suggestion: the "[^}]" could
>> be dropped here. that should get us back down to one character, but will
>> also filter the necessary keys. and anchors like "{key" are just as
>> invalid as "{key}" in asciidoc.
>>
> 
> 
> or move it to a single one replacing the whole non-greedy part, something like
> (untested): [^{}\[\]"']+
> 
should work to, yes. i used grep to test this and it worked (i did need
to do some escaping for the quotes, but the idea seems solid). i'll send
a v3 with this if you approve :)

Re: [pve-devel] [PATCH docs v2 2/2] asciidoc-pve: ignore anchor names in curly braces

[Thomas Lamprecht]

Am 14/11/2022 um 11:13 schrieb Stefan Sterz:
>> or move it to a single one replacing the whole non-greedy part, something like
>> (untested): [^{}\[\]"']+
>>
> should work to, yes. i used grep to test this and it worked (i did need
> to do some escaping for the quotes, but the idea seems solid). i'll send
> a v3 with this if you approve 😄
> 

would be fine for me, but no hard feeling on either; so go with what seems
the most reasonable for you.

[pve-devel] [PATCH manager v2 1/1] fix #4328: ui: add widget toolkit to dependencies of OnlineHelpInfo.js

[Stefan Sterz]

previously the widget toolkit was not scanned when creating the
mapping between `onlineHelp` keys and pve-doc anchors. this could
lead to cases where help buttons didn't work because the necessary
mapping wasn't present in `OnlineHelpInfo.js`.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
i took the liberty here to remove "/usr/bin/asciidoc-pve" as a
prerequisite of the affected make target. imo it isn't prerequisite
any more than "eslint" is for the `lint` target. it should already be
handled by the build dependencies anyway afaiu.

if that's not correct, i'll send a v3.

 debian/control        | 1 +
 www/manager6/Makefile | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/debian/control b/debian/control
index 35c9eee3..712d65e8 100644
--- a/debian/control
+++ b/debian/control
@@ -19,6 +19,7 @@ Build-Depends: debhelper (>= 12~),
                libtemplate-perl,
                libtest-mockmodule-perl,
                lintian,
+               proxmox-widget-toolkit (>= 3.4-9),
                pve-cluster,
                pve-container,
                pve-doc-generator (>= 7.0-4),
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 5938c7f5..2802cbac 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -288,6 +288,8 @@ JSSRC= 							\
 	Workspace.js					\
 # end of JSSRC list
 
+WIDGETKIT=/usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js
+
 all:
 
 .lint-incremental: ${JSSRC}
@@ -304,8 +306,8 @@ pvemanagerlib.js: .lint-incremental OnlineHelpInfo.js ${JSSRC}
 	cat OnlineHelpInfo.js ${JSSRC} >$@.tmp
 	mv $@.tmp $@
 
-OnlineHelpInfo.js: /usr/bin/asciidoc-pve ${JSSRC}
-	/usr/bin/asciidoc-pve scan-extjs ${JSSRC} >$@.tmp
+OnlineHelpInfo.js: ${JSSRC} ${WIDGETKIT}
+	/usr/bin/asciidoc-pve scan-extjs $^ >$@.tmp
 	mv $@.tmp $@
 
 .PHONY: install
-- 
2.30.2