Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-exclude without trailing space causes This namespace does not exist error #153

Closed
dregad opened this issue Sep 15, 2023 · 5 comments · Fixed by #155
Closed

-exclude without trailing space causes This namespace does not exist error #153

dregad opened this issue Sep 15, 2023 · 5 comments · Fixed by #155

Comments

@dregad
Copy link
Contributor

dregad commented Sep 15, 2023

I have a page with a complex nstables tag that requires multiple excludes. In the interest of making it easier to read & maintain, I split it over multiple lines like so:

<nspages -r -title -sortByDate -reverse -numberedList -textPages="" 
  -exclude:pv:modele_pv 
  -exclude:wiki: 
  -exclude:playground: 
  ... more excludes ...
>

If there is no space after any of the -exclude lines, I get e.g.

this namespace doesn't exist: -exclude:pv:wiki

Adding a trailing space fixes the problem. I would expect the newline to be treated like the space.

@dregad dregad changed the title -exclude without trailing space causes *This namespace does not exist* error -exclude without trailing space causes This namespace does not exist error Sep 15, 2023
@dregad dregad changed the title -exclude without trailing space causes This namespace does not exist error -exclude without trailing space causes This namespace does not exist error Sep 15, 2023
@dregad dregad changed the title -exclude without trailing space causes This namespace does not exist error -exclude without trailing space causes This namespace does not exist error Sep 15, 2023
@dregad
Copy link
Contributor Author

dregad commented Sep 15, 2023

Actually the same problem occurs after other options too

@dregad
Copy link
Contributor Author

dregad commented Sep 15, 2023

Looks like root cause is 1042ed4

dregad added a commit to dregad/dokuwiki-plugin-nspages that referenced this issue Sep 15, 2023
Previously, only space was considered. This caused error when the
nspages tag spanned multiple lines and there was no trailing space
before the newline.

Fixes gturri#153
@dregad
Copy link
Contributor Author

dregad commented Sep 15, 2023

Proposed fix in PR #154

gturri added a commit that referenced this issue Sep 15, 2023
The issue is not fixed yet, but at least we now already have a test
for when a fix will be available
@gturri gturri closed this as completed in b18078a Sep 15, 2023
@gturri
Copy link
Owner

gturri commented Sep 15, 2023

thanks for the bug report!
(Don't hesitate to re-open it if it appears the last version don't fix every occurrence of it.)

@dregad
Copy link
Contributor Author

dregad commented Sep 15, 2023

@gturri Thanks for the fast response. I have tested your fix.

The good news is that it does improve the situation, effectively handling the trailing \n scenario which was initially causing me problems, in addition to the original case with a regular space ( ).

But of course as you suspected in #154 (comment) it will fail again when the option is followed by any other kind of whitespace (e.g. \t or \r).

Of course you can add another str_replace() call for each possibility, but that does not feel right. I believe that my approach of using a preg_replace() call was cleaner and more generic.

The regression on the original issue #123 that my proposed fix introduced can easily be addressed by using a modified regex pattern (\s+ instead of \s*). I've tested that locally and the original issue <nspages -exclude:page1 -exclude:page10> seems to be correctly handled.

I can't reopen this issue myself, but I'll submit another PR so you can review and test.

dregad added a commit to dregad/dokuwiki-plugin-nspages that referenced this issue Sep 15, 2023
Previously, only space was considered. This caused error when the
nspages tag spanned multiple lines and there was no trailing space
before the newline.

Fixes gturri#153
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants