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

How to set catalog prefer as public in vscode setting? #295

Closed
mingxi opened this issue Jul 26, 2020 · 16 comments
Closed

How to set catalog prefer as public in vscode setting? #295

mingxi opened this issue Jul 26, 2020 · 16 comments
Assignees
Labels
enhancement New feature or request invalid This doesn't seem right
Milestone

Comments

@mingxi
Copy link

mingxi commented Jul 26, 2020

Description
Is possible to set catalog prefer as public from vscode setting?
It seems that vscode-xml always set catalog as system id.
As far as I know, I can set catalog using CatalogManager.properties in java code.
I did not find the corresponding setting(prefer=public) in vscode-xml.
The below code is CatalogManager.properties which used to validate xml in java code.
e.g.

catalogs=C:\\DITA-OT\\catalog-dita.xml
relative-catalogs=yes
#verbosity=99
prefer=public
static-catalog=yes
allow-oasis-xml-catalog-pi=yes

Reference
https://xerces.apache.org/xml-commons/components/apidocs/resolver/org/apache/xml/resolver/CatalogManager.html

@angelozerr
Copy link
Contributor

angelozerr commented Jul 28, 2020

This feature is indeed not supported. To support it, XML Language Server (LemMinx) must support it. The basic idea is after creating the XMLCatalogResolver at https://github.com/eclipse/lemminx/blob/b1407bb52ce3ff6cd350a98b9c5aa8ea3392df0e/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/uriresolver/XMLCatalogResolverExtension.java#L121

we must configure prefer public with https://xerces.apache.org/xerces2-j/javadocs/xerces2/org/apache/xerces/util/XMLCatalogResolver.html#setPreferPublic(boolean)

I think we should support too UseLiteralSystemId with https://xerces.apache.org/xerces2-j/javadocs/xerces2/org/apache/xerces/util/XMLCatalogResolver.html#setUseLiteralSystemId(boolean)

We must find a settings name

Today we have xml.catalogs to define XML catalogs files which is an array of String:

{
	"xml": {
		"catalogs": [
			"./catalog1.xml"
		]
	}
}

I wonder if we should change this settings with xml.catalog.files to have that:

{
	"xml": {
		"catalog": {
			"preferPublic": true,
			"useLiteralSystemId": true, 
			"files": [
				"./catalog1.xml"
			]
		}
	}
}

I mean in vscode-xml:

  • remove xml.catalogs settings
  • add xml.catalog.preferPublic settings (boolean)
  • add xml.catalog.useLiteralSystemId settings (boolean)

@fbricon what do you think about that?

@mingxi
Copy link
Author

mingxi commented Jul 28, 2020

@angelozerr Got it, thanks for your reply.

@fbricon fbricon added this to the 0.14.0 milestone Aug 11, 2020
@fbricon fbricon added the enhancement New feature or request label Aug 11, 2020
angelozerr added a commit to angelozerr/lemminx that referenced this issue Aug 13, 2020
angelozerr added a commit to angelozerr/lemminx that referenced this issue Aug 17, 2020
angelozerr added a commit to angelozerr/vscode-xml that referenced this issue Aug 17, 2020
@fbricon
Copy link
Collaborator

fbricon commented Aug 17, 2020

I wonder if we should change this settings with xml.catalog.files to have that:

{
	"xml": {
		"catalog": {
			"preferPublic": true,
			"useLiteralSystemId": true, 
			"files": [
				"./catalog1.xml"
			]
		}
	}
}

LGTM

I mean in vscode-xml:

  • remove xml.catalogs settings

We prolly want to deprecate it first

@angelozerr
Copy link
Contributor

Thanks @fbricon for your feedback.

@mingxi I have implemented this issue but I don't understand how prefer feature is working and I have not found some documentation which explains how it works.

Could you please add a simple usecase with prefer (public and system) by given a sample with catalog, dtd, and explain the usecase that you wish to manage. Thanks!

@mingxi
Copy link
Author

mingxi commented Aug 18, 2020

@fbricon @angelozerr Thank you very much, I will test this feature asap.

@angelozerr
Copy link
Contributor

@fbricon @angelozerr Thank you very much, I will test this feature asap.

It's not available for the moment (my work is not merged). I would like to know play with my PR before merging my work, but I need to understand how prefer is working.

@mingxi
Copy link
Author

mingxi commented Aug 18, 2020

feature

Okay, I will sort it out and I will share the test cases

angelozerr added a commit to angelozerr/lemminx that referenced this issue Aug 18, 2020
@mingxi
Copy link
Author

mingxi commented Aug 18, 2020

@angelozerr
Step:

  1. You can download the latest DITA-OT from https://github.com/dita-ot/dita-ot/releases/download/3.5.1/dita-ot-3.5.1.zip
  2. In my side, unzip it into C:\Users\mingxi\Downloads\dita-ot-3.5.1
  3. Set xml.catalog.files as below:
{
	"xml": {
		"catalog": {
			"preferPublic": true,
			"useLiteralSystemId": false, 
			"files": [
				"C:\\Users\\mingxi\\Downloads\\dita-ot-3.5.1\\catalog-dita.xml"
			]
		}
	}
}`
4. Open C:\Users\mingxi\Downloads\dita-ot-3.5.1\docsrc\site.ditamap by VSCode and validate it.

angelozerr added a commit to angelozerr/vscode-xml that referenced this issue Aug 18, 2020
@mingxi
Copy link
Author

mingxi commented Aug 18, 2020

image
image
@angelozerr
Please see above screenshot.
I downloaded your commit and replaced the package.json in extension folder with your package.json

@mingxi
Copy link
Author

mingxi commented Aug 18, 2020

@angelozerr

Thanks I will test it.
FYI
Below is my code to validate xml manually.

XmlTool.java

import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.StringReader;
import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
import java.util.List;
import org.apache.xerces.util.XMLCatalogResolver;
import org.apache.xml.resolver.tools.CatalogResolver;
import org.dom4j.io.SAXReader;


public class XmlTool {

    private StringBuilder validateXML(String xml) {
        StringBuilder error = new StringBuilder();
        SAXReader saxReader = new SAXReader();
        // use catalog xml to map dtd
        CatalogResolver resolver = new CatalogResolver(true);
        XMLCatalogResolver xmlResolver = createXMLCatalogResolver(resolver);
        saxReader.setEntityResolver(xmlResolver);
        saxReader.setValidation(true);
        try {
            saxReader.read(new StringReader(xml));
        } catch (Exception e) {
            error.append(e.getMessage());
        }

        return error;
    }

    private static XMLCatalogResolver createXMLCatalogResolver(CatalogResolver resolver) {
        int index = 0;
        List files = resolver.getCatalog().getCatalogManager().getCatalogFiles();
        String catalogs[] = new String[files.size()];
        for (Object file : files) {
            try {
                String catalogPath = new File(file.toString()).toURL().toExternalForm();
                catalogs[index] = catalogPath;
            } catch (MalformedURLException e) {
                e.printStackTrace();;
            }
            index++;
        }
        XMLCatalogResolver xmlResolver = new XMLCatalogResolver();
        // xmlResolver.setPreferPublic(true);
        xmlResolver.setCatalogList(catalogs);

        return xmlResolver;
    }

    private static String readfile(String filePath) throws IOException {
		StringBuilder sbfFileContents = new StringBuilder();
		InputStream is = null;
		try {
			is = new FileInputStream(filePath);
			BufferedReader bReader = new BufferedReader(new InputStreamReader(
					is, "utf-8"));

			String line = null;
			while ((line = bReader.readLine()) != null) {
				sbfFileContents.append(line);
			}
		} catch (FileNotFoundException e) {
			e.printStackTrace();
		} catch (UnsupportedEncodingException e) {
			e.printStackTrace();
		} catch (IOException e) {
			e.printStackTrace();
		} finally {
			if (null != is)
				is.close();
		}

		return sbfFileContents.toString();
	}

    public static void main(String argv[]) throws Exception {
            XmlTool test = new XmlTool();
            String xmlContent = XmlTool.readfile("C:\\Users\\mgao26\\Downloads\\dita-ot-3.5.1\\docsrc\\site.ditamap");
            StringBuilder error = test.validateXML(xmlContent);
            if (error.length() == 0) {
                System.out.println("Pass");
            } else {
                System.out.println(error);
            }
        }
}

CatalogManager.properties

catalogs=C:\\Users\\mgao26\\Downloads\\dita-ot-3.5.1\\catalog-dita.xml
relative-catalogs=yes
#verbosity=99
prefer=public
static-catalog=yes
allow-oasis-xml-catalog-pi=yes

angelozerr added a commit to angelozerr/lemminx that referenced this issue Aug 18, 2020
@angelozerr
Copy link
Contributor

Thanks @mingxi for your feedback, the problem doesn't come from XML catalog configuration (prefer, etc) but comes from several bugs when you use XML catalog:

I have fixed that in my local labtop, now I must create clean PR. As soon as I will have something to test I will ping you.

@angelozerr
Copy link
Contributor

I downloaded your commit and replaced the package.json in extension folder with your package.json

It's not enough, you need to compile XML Language Server (LemMinx), see https://github.com/redhat-developer/vscode-xml/blob/master/CONTRIBUTING.md

@angelozerr
Copy link
Contributor

@mingxi could please:

Validation, XML compltion, hover based on DTD should work:

image

Pease give me feedback, thanks!

@mingxi
Copy link
Author

mingxi commented Aug 24, 2020

@angelozerr Thanks a lot, works fine for me.

image

@mingxi
Copy link
Author

mingxi commented Aug 24, 2020

@angelozerr You can close it now, thank you very much.

@angelozerr
Copy link
Contributor

@angelozerr You can close it now, thank you very much.

Glad it works for you! Don't hesitate to create another issues if you find some problem or you require some improvement.

If you need really support settings like prefer (public, system), please reopen the issue by giving a usecase.

I close this issue even if prefer settings is not supported since it was a bug which was fixed.

@angelozerr angelozerr added the invalid This doesn't seem right label Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants