Code Monkey home page Code Monkey logo

Comments (32)

luigiberrettini avatar luigiberrettini commented on June 28, 2024 1

Closing since the misbehavior is not caused by the target.

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024 1

The best solution would be to use the power of XML adding an assembly optional attribute:

<?xml version="1.0" encoding="utf-8"?>
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xmlns:sl="http://www.nlog-project.org/schemas/NLog.Targets.Syslog.xsd">
    <targets>
        <target xsi:type="sl:Syslog" assembly="NLog.Targets.Syslog" name="syslog-tgt">
            <!-- ... -->
        </target>
    </targets>
    <!-- ... -->
</nlog>

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024 1

The important thing is just to declare it in the docs, then you can even diverge a lot more from the standard.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

Looks like I was not thorough enough, when adding support for embedded assembly-name in type-alias. I liked the dotnet-syntax of specifying the assembly-name. But seems it is not compatible with QName-format.

Maybe adding support for assembly-name as QName-prefix like this:

<target xsi:type="NLog.Targets.Syslog:Syslog" name="cee-udp">

See also: NLog/NLog#4979

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

Curious is the XML Schema validation is still able to validate properties for the SysLog-target when using NLog.Targets.Syslog: as prefix?

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

@davidgeary In your example you use xsi:type="sl:syslog", any reason why you include the sl:-prefix ?

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

Maybe it comes from these examples:

https://github.com/luigiberrettini/NLog.Targets.Syslog/blob/master/src/TestAppWithTUI/NLog.config
https://github.com/luigiberrettini/NLog.Targets.Syslog/blob/master/src/TestAppWithGUI/NLog.config

Curious what is expected from xsi:type="sl:syslog" instead of just xsi:type="syslog" ?

from nlog.targets.syslog.

davidgeary avatar davidgeary commented on June 28, 2024

In your example you use xsi:type="sl:syslog", any reason why you include the sl:-prefix ?

The precise reason is lost in the mists of time, but best guess is that I just copied it from an example somewhere :-)

I've just tried replacing sl: with NLog.Targets.Syslog: in the type property and it works the same as long as the assembly is still included after the type name. Just using the prefix without appending the assembly name fails to load it, but that's probably to be expected, since the <target> element is NLog-specific (rather than part of the XML std), so the format of the type property will surely need to conform to the formats supported by NLog?

AFAICS, the only place they mention the new capability of including the assembly name is in the list of NLog 5.0 changes:

This means custom targets requires explicit <add assembly="NLog.MyAssembly" />, or use the new ability to specify type with assembly-name: <target type="MyTarget, NLog.MyAssembly" name="hello" />.

This would suggest that the assembly must follow the type, separated by a comma.

(Was about to suggest getting clarification from NLog, but have just seen the issue you've raised, so will add a comment to that.)

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

Thank you for confirming that intellisense also works for you when using prefix NLog.Targets.Syslog:. And also thank you for confirming that there is no clear reason for using sl: as prefix.

Now just need to decide if it should be NLog 5.0.2 or NLog 5.1 for using xsi:type qname-prefix as hint for assembly-loading (So XSD-intellisense will work together with assembly-loading-hint)

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

Curious what is expected from xsi:type="sl:syslog" instead of just xsi:type="syslog"?

The sl: prefix is used to comply with the specifications by providing a QName as the xsi:type attribute expects.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

The sl: prefix is used to comply with the specifications by providing a QName as the xsi:type attribute expects.

Pretty sure the QName-prefix is optional.

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

Pretty sure the QName-prefix is optional

Yes, like for attribute names.
I just answered to clarify the doubts about its usage and to highlight the fact is has to be supported regardless of being optional or not.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

highlight the fact is has to be supported regardless of being optional or not.

Yes currently NLog puts a lot of effort into stripping away the prefix, so was just curious what benefit the prefix had.

Was maybe thinking to support this format xsi:type="NLog.Targets.Syslog:Syslog", where NLog will use the prefix as Assembly-loading-hint.

from nlog.targets.syslog.

davidgeary avatar davidgeary commented on June 28, 2024

Closing since the misbehavior is not caused by the target.

But this issue wasn't about the target specifically; it was to do with the XML schema, which identifies the new format as invalid. (The original title of this issue was "XML schema warning issued when specifying extension assembly via target's type property".)

The warning incorrectly gives the impression that including the assembly name in the type property is invalid, when in reality, it is the schema that is wrong, since NLog no longer requires the type property to be a QName.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

But this issue wasn't about the target specifically; it was to do with the XML schema, which identifies the new format as invalid.

The xsi:type is not part of the XML-Schema from NLog.xsd or NLog.Targets.Syslog.xsd.

The problem is not the XML-schema, but the NLog-team having broken the XSD-xsi-standard, by introducing a non-standard format which doesn't work with the xsi:type-attribute.

Guess we need more space-cowboys from the 2000's that knows XML / XSD / XSLT by heart on the NLog-team, but until that happens, then we just have to go along with what we have, and repair as we go. See also NLog/NLog#4979

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

It seems from Wikipedia that the QName syntax is Prefix:LocalPart where both Prefix and LocalPart are NCNames.

An NCName value follows these rules:

  • it does not contain a colon
  • it must consist exclusively of letters, digits, ideographs, underscores, hyphens, periods
  • it cannot start with a digit, an hyphen, the period

This means that NLog should support one of the two options below, where sl:Syslog_NLog.Targets.Syslog is just a possible choice to comply with NCName syntax.

OPTION 1

<?xml version="1.0" encoding="utf-8"?>
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xmlns:sl="http://www.nlog-project.org/schemas/NLog.Targets.Syslog.xsd">
    <extensions>
        <add assembly="NLog.Targets.Syslog"/>
    </extensions>
    <targets>
        <target xsi:type="sl:Syslog" name="syslog-tgt">
            <!-- ... -->
        </target>
    </targets>
    <!-- ... -->
</nlog>

OPTION 2

<?xml version="1.0" encoding="utf-8"?>
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xmlns:sl="http://www.nlog-project.org/schemas/NLog.Targets.Syslog.xsd">
    <targets>
        <target xsi:type="sl:Syslog_NLog.Targets.Syslog" name="syslog-tgt">
            <!-- ... -->
        </target>
    </targets>
    <!-- ... -->
</nlog>

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

@luigiberrettini Any reason why xsi:type="NLog.Targets.Syslog:Syslog" is bad ? (Since XSD-intellisense and XSD-validation will continue to work with that format)

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

Because prefix should be an XML namespace declared in the file like xsi and sl and using NLog.Targets.Syslog as a prefix would not allow to specify the sl namespace.

In my opinion the goal should be both complying with specs and avoiding validation warnings/errors.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

When using <target xsi:type="sl:Syslog_NLog.Targets.Syslog" name="syslog-tgt"> then XSD-intellisense and XSD-Validation stops working.

But with xsi:type="NLog.Targets.Syslog:Syslog" then everything is great. Again have no clue about what meaning the "prefix" has since it seems to be ignored by XSD-tooling.

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

Ok then we need to find something that respects the specs and can be validated with a proper XML validator and then see if the Visual Studio validator works properly.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

What about xsi:type="NLog.Targets.Syslog:Syslog" ? (Again it is only an option that you can use, not something that is required if you like to use <extensions> instead)

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

As I said that does not respects the specs.

Having as a goal to remove warnings will result in somebody else opening an issue because it is not XML compliant and not solving both is just saying, use another option, in which case this could have been the reply to this issue also.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

How does the QName-prefix NLog.Targets.Syslog: break the specs and the tooling ?

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

As I wrote above the prefix must be an XML namespace and it is not.

I found out that xsi:type="sl:Syslog_NLog.Targets.Syslog" works but the NLog XSD needs to be changed: sl:* are identifiers looked up into the XSD.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

assembly is not a "reserved"-word like xsi:type

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

I do not know if it is reserved, to me you can use whatever word you prefer: you just need to add it in the NLog XSD specification so that it is recognized as attribute of the target element.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

Yes that is also a possible way for making assembly-loading-hint work with xsi:type-attribute without validation-errors.

Though I prefer xsi:type="NLog.Targets.Syslog:Syslog" (Since NLog.config never promised to follow XSD-standard, but like the help from the tooling with validation and intellisense)

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

Still I believe that a separate attribute in the XSD Target abstract complex type would be cleaner, more efficient than splitting for parsing purposes and could allow to deprecate the extensions altogether.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

<extensions> are still needed for layout-renderers extensions when parsing NLog Layout. Having the assembly-loading-hint was just a random idea to make it easier for NLog-users to use NLog-target-extensions, but also a bridge to allow NLog-core-project to eject NLog-targets that was specific to Microsof Windows into their own nuget-packages.

Can now see that my initial idea was a little better (xsi:type="sl:NLog.Targets.Syslog.Syslog") according to XML-standards, and it was only the suggestion from @ThomasArdal that made it break XML-standards for QName. But XSD-intellisense would still be broken

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

One not so nice solution but working and complying with specs:

<?xml version="1.0" encoding="utf-8"?>
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xmlns:NLog.Targets.Syslog="http://www.nlog-project.org/schemas/NLog.Targets.Syslog.xsd">
    <targets>
        <target xsi:type="NLog.Targets.Syslog:Syslog" name="syslog-tgt">
            <NLog.Targets.Syslog:layout xsi:type="SimpleLayout" text="${message}" />
            <!-- ... -->
        </target>
    </targets>
    <!-- ... -->
</nlog>

from nlog.targets.syslog.

luigiberrettini avatar luigiberrettini commented on June 28, 2024

xsi:type="sl:NLog.Targets.Syslog.Syslog"

Considering that you want to support both extensions plus target name and no extensions plus assembly plus target name there is no solution working since target XSDs only have one or the other.

I have Syslog in mine and any change would imply me changing the name in the XSD, making the XSD NuGet package useless unless you force all target implementers to always use the full name.

from nlog.targets.syslog.

snakefoot avatar snakefoot commented on June 28, 2024

The idea was not to force anything, but just to make it easier to provide an assembly-loading-hint with the type-alias.

But see that you have to navigate careful in the woods of XML and xmlns. Guess @davidgeary should stay away from using xsi:type with assembly-loading-hint (And stick with <extensions>), until a white knight arrives and clears everything up,.

Alternative replace xsi:type="sl:Syslog, NLog.Targets.Syslog with type="Syslog, NLog.Targets.Syslog", but then loose XML-validation and XML-intellisense.

from nlog.targets.syslog.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.