Question: uniqueness of keys in Attr

Question: uniqueness of keys in Attr

[Zev Spitz]

[-- Attachment #1.1: Type: text/plain, Size: 635 bytes --]

Text.Pandoc.Definitions defines an Attr type whose third element is a list 
of key-value pairs.
Are these keys guaranteed to be unique? Is there any reason why this isn't 
represented as a Map Text Text?

Thanks in advance.

-- 
You received this message because you are subscribed to the Google Groups "pandoc-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pandoc-discuss+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web visit https://groups.google.com/d/msgid/pandoc-discuss/642eea4f-58bb-4533-ad47-e97f20bd4105n%40googlegroups.com.

[-- Attachment #1.2: Type: text/html, Size: 967 bytes --]

Re: Question: uniqueness of keys in Attr

[John MacFarlane]

No, there's no uniqueness guarantee.

I think the main reason a map wasn't used is that we wanted
to preserve order.  But it's probably not the best
representation.


Zev Spitz <spitzzev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> Text.Pandoc.Definitions defines an Attr type whose third element is a list 
> of key-value pairs.
> Are these keys guaranteed to be unique? Is there any reason why this isn't 
> represented as a Map Text Text?
>
> Thanks in advance.
>
> -- 
> You received this message because you are subscribed to the Google Groups "pandoc-discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to pandoc-discuss+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> To view this discussion on the web visit https://groups.google.com/d/msgid/pandoc-discuss/642eea4f-58bb-4533-ad47-e97f20bd4105n%40googlegroups.com.

Re: Question: uniqueness of keys in Attr

[Gwern Branwen]

Is there a reason uniqueness is not enforced & order is preserved?
(Aside from lists being a lot easier to work with, and at this point,
historical/backwards compatibility, of course!)

As far as I can tell for SGML, XML, HTML etc it's generally the case
that duplicate attribute keys are considered an error. (Browsers
presumably do something like take the first key when dealing with
invalid HTML, which is dangerous since keys might be reordered or the
user write in varying orders.) For HTML5
https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 the
spec goes beyond that and says there cannot be any case-insensitive
duplicates:

> There must never be two or more attributes on the same start tag whose names are an ASCII case-insensitive match for each other.

Seems like, if there is no valid reason to emit duplicate keys, the
HTML-generating code should be erroring out or dropping the duplicates
while emitting a warning, rather than generating invalid HTML.

-- 
gwern
https://www.gwern.net

Re: Question: uniqueness of keys in Attr

[John MacFarlane]

Agreed that it would be better in principle to enforce
uniqueness in the type.  And also in principle, one could
do that and enforce order.

https://hackage.haskell.org/package/ordered-containers-0.2.2/docs/Data-Map-Ordered.html

This is one of many things I'd have done differently if I
were doing it now.  But changing this now would be quite a lot of
work, and the benefit isn't so huge.

We could also just change the HTML writer to do something about
duplicate keys.

Gwern Branwen <gwern-v26ZT+9V8bxeoWH0uzbU5w@public.gmane.org> writes:

> Is there a reason uniqueness is not enforced & order is preserved?
> (Aside from lists being a lot easier to work with, and at this point,
> historical/backwards compatibility, of course!)
>
> As far as I can tell for SGML, XML, HTML etc it's generally the case
> that duplicate attribute keys are considered an error. (Browsers
> presumably do something like take the first key when dealing with
> invalid HTML, which is dangerous since keys might be reordered or the
> user write in varying orders.) For HTML5
> https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 the
> spec goes beyond that and says there cannot be any case-insensitive
> duplicates:
>
>> There must never be two or more attributes on the same start tag whose names are an ASCII case-insensitive match for each other.
>
> Seems like, if there is no valid reason to emit duplicate keys, the
> HTML-generating code should be erroring out or dropping the duplicates
> while emitting a warning, rather than generating invalid HTML.
>
> -- 
> gwern
> https://www.gwern.net
>
> -- 
> You received this message because you are subscribed to the Google Groups "pandoc-discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to pandoc-discuss+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> To view this discussion on the web visit https://groups.google.com/d/msgid/pandoc-discuss/CAMwO0gwetbPYPw6-zGr1O9d_bTsufP32OZ-Oe00k-Rzdi6qFhg%40mail.gmail.com.

Re: Question: uniqueness of keys in Attr

[Zev Spitz]

[-- Attachment #1.1: Type: text/plain, Size: 3259 bytes --]

The issue I am trying to address is: in the interface I've written for 
Pandoc filters <https://github.com/zspitz/PandocFilters>, I represent Attr 
as an ordered list of 2-string tuples 
<https://github.com/zspitz/PandocFilters/blob/main/PandocFilters/Ast/Types.cs#L327>. 
I am wondering if another data type would be more appropriate.

   1. Is it safe to use unique-key-enforcing type? From the discussion, I 
   understand the answer is "yes", because keys are unique for all handled 
   document formats.
   2. Is there any point in using an ordered type? After all, the AST is 
   converted to JSON, whose object type doesn't enforce any sort of order.
   

On Friday, March 5, 2021 at 4:57:54 AM UTC+2 John MacFarlane wrote:

>
> Agreed that it would be better in principle to enforce
> uniqueness in the type. And also in principle, one could
> do that and enforce order.
>
>
> https://hackage.haskell.org/package/ordered-containers-0.2.2/docs/Data-Map-Ordered.html
>
> This is one of many things I'd have done differently if I
> were doing it now. But changing this now would be quite a lot of
> work, and the benefit isn't so huge.
>
> We could also just change the HTML writer to do something about
> duplicate keys.
>
> Gwern Branwen <gw...-v26ZT+9V8bxeoWH0uzbU5w@public.gmane.org> writes:
>
> > Is there a reason uniqueness is not enforced & order is preserved?
> > (Aside from lists being a lot easier to work with, and at this point,
> > historical/backwards compatibility, of course!)
> >
> > As far as I can tell for SGML, XML, HTML etc it's generally the case
> > that duplicate attribute keys are considered an error. (Browsers
> > presumably do something like take the first key when dealing with
> > invalid HTML, which is dangerous since keys might be reordered or the
> > user write in varying orders.) For HTML5
> > https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 the
> > spec goes beyond that and says there cannot be any case-insensitive
> > duplicates:
> >
> >> There must never be two or more attributes on the same start tag whose 
> names are an ASCII case-insensitive match for each other.
> >
> > Seems like, if there is no valid reason to emit duplicate keys, the
> > HTML-generating code should be erroring out or dropping the duplicates
> > while emitting a warning, rather than generating invalid HTML.
> >
> > -- 
> > gwern
> > https://www.gwern.net
> >
> > -- 
> > You received this message because you are subscribed to the Google 
> Groups "pandoc-discuss" group.
> > To unsubscribe from this group and stop receiving emails from it, send 
> an email to pandoc-discus...-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> > To view this discussion on the web visit 
> https://groups.google.com/d/msgid/pandoc-discuss/CAMwO0gwetbPYPw6-zGr1O9d_bTsufP32OZ-Oe00k-Rzdi6qFhg%40mail.gmail.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups "pandoc-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pandoc-discuss+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web visit https://groups.google.com/d/msgid/pandoc-discuss/05100c29-a18b-4a17-be12-8f14d6522dc0n%40googlegroups.com.

[-- Attachment #1.2: Type: text/html, Size: 5418 bytes --]

Re: Question: uniqueness of keys in Attr

[John MacFarlane]

Zev Spitz <spitzzev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> The issue I am trying to address is: in the interface I've written for 
> Pandoc filters <https://github.com/zspitz/PandocFilters>, I represent Attr 
> as an ordered list of 2-string tuples 
> <https://github.com/zspitz/PandocFilters/blob/main/PandocFilters/Ast/Types.cs#L327>. 
> I am wondering if another data type would be more appropriate.
>
>    1. Is it safe to use unique-key-enforcing type? From the discussion, I 
>    understand the answer is "yes", because keys are unique for all handled 
>    document formats.

It depends on what you want to do with it.  Internally, pandoc
represents Attr as a list of tuples.  So you'll have to change it
back into this form when converting to pandoc JSON.  If you
use a map internally, then the order of attributes may change
when you do this, and that may lead to unexpected (but probably
not very important) changes in the output.

>    2. Is there any point in using an ordered type? After all, the AST is 
>    converted to JSON, whose object type doesn't enforce any sort of order.

Right, but Attr is represented as an array of arrays, not an
object.

Re: Question: uniqueness of keys in Attr

[Gwern Branwen]

On Thu, Mar 4, 2021 at 9:57 PM John MacFarlane <jgm-TVLZxgkOlNX2fBVCVOL8/A@public.gmane.org> wrote:
> We could also just change the HTML writer to do something about
> duplicate keys.

How about emitting a warning, something like:

diff --git a/src/Text/Pandoc/Writers/HTML.hs b/src/Text/Pandoc/Writers/HTML.hs
index 26df0325e..bbadcee32 100644
--- a/src/Text/Pandoc/Writers/HTML.hs
+++ b/src/Text/Pandoc/Writers/HTML.hs
@@ -30,7 +30,7 @@ module Text.Pandoc.Writers.HTML (
   ) where
 import Control.Monad.State.Strict
 import Data.Char (ord)
-import Data.List (intercalate, intersperse, partition, delete, (\\))
+import Data.List (intercalate, intersperse, partition, delete, nub, sort, (\\))
 import Data.List.NonEmpty (NonEmpty((:|)))
 import Data.Maybe (fromMaybe, isJust, isNothing, mapMaybe)
 import qualified Data.Set as Set
@@ -572,6 +572,11 @@ toAttrs kvs = do
 attrsToHtml :: PandocMonad m
             => WriterOptions -> Attr -> StateT WriterState m [Attribute]
 attrsToHtml opts (id',classes',keyvals) = do
+  -- warn on non-unique (case-insensitive) keys, which are invalid
HTML5: https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
+  -- > There must never be two or more attributes on the same start
tag whose names are an ASCII case-insensitive match for each other.
+  let uniqueKeys = nub $ sort $ map (T.toCaseFold . fst) keyvals
+  when (length uniqueKeys /= length keyvals) $ report $
IgnoredElement (T.concat uniqueKeys)
+
   attrs <- toAttrs keyvals
   return $
     [prefixedId opts id' | not (T.null id')] ++

(I didn't find any good logging type in Logging.hs so I picked
IgnoredElement and concatenated the keys pretty arbitrarily. Maybe you
can add an 'InvalidHTML' constructor or something.)

-- 
gwern
https://www.gwern.net

Re: Question: uniqueness of keys in Attr

[John MacFarlane]

It's a good idea. For better performance, I'll do this in toAttrs
and replace the mapMaybe with a foldM.  The fold, when processing
each attribute, can check for duplication and issue a warning.

Gwern Branwen <gwern-v26ZT+9V8bxeoWH0uzbU5w@public.gmane.org> writes:

> On Thu, Mar 4, 2021 at 9:57 PM John MacFarlane <jgm-TVLZxgkOlNX2fBVCVOL8/A@public.gmane.org> wrote:
>> We could also just change the HTML writer to do something about
>> duplicate keys.
>
> How about emitting a warning, something like:
>
> diff --git a/src/Text/Pandoc/Writers/HTML.hs b/src/Text/Pandoc/Writers/HTML.hs
> index 26df0325e..bbadcee32 100644
> --- a/src/Text/Pandoc/Writers/HTML.hs
> +++ b/src/Text/Pandoc/Writers/HTML.hs
> @@ -30,7 +30,7 @@ module Text.Pandoc.Writers.HTML (
>    ) where
>  import Control.Monad.State.Strict
>  import Data.Char (ord)
> -import Data.List (intercalate, intersperse, partition, delete, (\\))
> +import Data.List (intercalate, intersperse, partition, delete, nub, sort, (\\))
>  import Data.List.NonEmpty (NonEmpty((:|)))
>  import Data.Maybe (fromMaybe, isJust, isNothing, mapMaybe)
>  import qualified Data.Set as Set
> @@ -572,6 +572,11 @@ toAttrs kvs = do
>  attrsToHtml :: PandocMonad m
>              => WriterOptions -> Attr -> StateT WriterState m [Attribute]
>  attrsToHtml opts (id',classes',keyvals) = do
> +  -- warn on non-unique (case-insensitive) keys, which are invalid
> HTML5: https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
> +  -- > There must never be two or more attributes on the same start
> tag whose names are an ASCII case-insensitive match for each other.
> +  let uniqueKeys = nub $ sort $ map (T.toCaseFold . fst) keyvals
> +  when (length uniqueKeys /= length keyvals) $ report $
> IgnoredElement (T.concat uniqueKeys)
> +
>    attrs <- toAttrs keyvals
>    return $
>      [prefixedId opts id' | not (T.null id')] ++
>
> (I didn't find any good logging type in Logging.hs so I picked
> IgnoredElement and concatenated the keys pretty arbitrarily. Maybe you
> can add an 'InvalidHTML' constructor or something.)
>
> -- 
> gwern
> https://www.gwern.net
>
> -- 
> You received this message because you are subscribed to the Google Groups "pandoc-discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to pandoc-discuss+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> To view this discussion on the web visit https://groups.google.com/d/msgid/pandoc-discuss/CAMwO0gyagC8w6ND2z0T0rDUVAo8Mpba87wff%3DYvXhgkg0S5Aaw%40mail.gmail.com.

Re: Question: uniqueness of keys in Attr

[Gwern Branwen]

On Wed, Mar 10, 2021 at 12:44 PM John MacFarlane <jgm-TVLZxgkOlNX2fBVCVOL8/A@public.gmane.org> wrote:
> It's a good idea. For better performance, I'll do this in toAttrs
> and replace the mapMaybe with a foldM.  The fold, when processing
> each attribute, can check for duplication and issue a warning.

Sounds good. One thing I forgot: to keep the performance impact as
near-zero as possible and improve the clarity of what is being
checked, I'd also include a shortcut for the cases where there's zero
or 1 key-value pairs, where duplication of keys is impossible, as
AFAIK that will be like 99% of most Pandoc users' links (I use
key-values heavily for my popups and local link archives, but most
Pandoc documents I've looked at use none at all).

-- 
gwern
https://www.gwern.net

Re: Question: uniqueness of keys in Attr

[John MacFarlane]

I've added the feature.

I don't think there should be any significant performance impact,
but I'll measure.

Gwern Branwen <gwern-v26ZT+9V8bxeoWH0uzbU5w@public.gmane.org> writes:

> On Wed, Mar 10, 2021 at 12:44 PM John MacFarlane <jgm-TVLZxgkOlNX2fBVCVOL8/A@public.gmane.org> wrote:
>> It's a good idea. For better performance, I'll do this in toAttrs
>> and replace the mapMaybe with a foldM.  The fold, when processing
>> each attribute, can check for duplication and issue a warning.
>
> Sounds good. One thing I forgot: to keep the performance impact as
> near-zero as possible and improve the clarity of what is being
> checked, I'd also include a shortcut for the cases where there's zero
> or 1 key-value pairs, where duplication of keys is impossible, as
> AFAIK that will be like 99% of most Pandoc users' links (I use
> key-values heavily for my popups and local link archives, but most
> Pandoc documents I've looked at use none at all).
>
> -- 
> gwern
> https://www.gwern.net