[For merging on 30.09] landing page is canonical #513

mjpost · 2019-08-18T01:08:14Z

This fix redirects paper pages like

https://aclweb.org/anthology/papers/P/P19/P19-1002/

to the canonical slashed variant

https://aclweb.org/anthology/P19-1002/

I hope this will stop people from accidentally sharing the nested URL forms which I find ugly and redundant. Note that this is just a small change; it's related to #480 but doesn't yet change the canonical URL. (Also note that I have tested this out and it is actually already in place).

mjpost · 2019-08-18T13:21:54Z

The latest push removes the links to nested pages in favor of the canonical variants:

paper page: ID/
BibTeX: ID.bib
EndNote: ID.endf

etc. I did not switch the canonical page yet. Testing shows that it works so far.

mjpost

I just submitted comments that should make the changes here live. Can someone from @acl-org/anthology take a look and approve?

hugo/layouts/papers/list-entry.html

mjpost · 2019-08-31T15:46:49Z

hugo/layouts/papers/list-entry.html

    {{- end -}}
    {{- $bibfile := printf "/papers/%s/%s/%s.bib" (slicestr $volume_id 0 1) $volume_id .Params.anthology_id -}}
    {{- if (fileExists (printf "/data-export/%s" $bibfile)) -}}
-    <a class="badge badge-secondary align-middle mr-1" href="{{ $bibfile | relURL }}" data-toggle="tooltip" data-placement="top" title="Export to BibTeX">
+    <a class="badge badge-secondary align-middle mr-1" href="{{ $paper.url }}.bib" data-toggle="tooltip" data-placement="top" title="Export to BibTeX">
      bib


The bib path is now ${ANTH_ID}.bib, e.g., https://aclweb.org/anthology/P19-1002.bib, instead of the nested version.

You're no longer using $bibfile, but the link is still within an if clause that checks for $bibfile.

This is fine though, right? If the bibfile exists, we generate the appropriate link to it.

I didn't change this because I'm not sure how to fix it. If the bibfile is present, we generate the canonical link to it, which is handled by a redirect internally.

So, the question is whether we still want a check for the existence of the file in the Hugo template.

If we do, then this current solution is suboptimal IMO since it requires us manually remembering and updating the actual local path to the file (in the $bibfile := ... line), in case it ever changes.

If we do not, we should just throw out the surrounding lines altogether.

mjpost · 2019-08-31T15:47:23Z

hugo/layouts/papers/list-entry.html

@@ -28,7 +25,7 @@
    {{ if eq hugo.Environment "development" }}
      <span class="badge badge-light align-middle">{{ .Params.anthology_id }}</span>
    {{ end }}
-    <strong><a class="align-middle" href="{{ .RelPermalink }}">{{ $paper.title_html | safeHTML }}</a></strong>
+    <strong><a class="align-middle" href="{{ $paper.url }}/">{{ $paper.title_html | safeHTML }}</a></strong>
    <br />


Link to the canonical page (https://aclweb.org/anthology/P19-1002/) instead of the nested version. This will get rid of the double search results.

Same concern as above: URL can refer to external content.

Fixed. url now points to the page's Anthology URL; pdf points to the PDF (which can be internal or external).

The downside of how this is now handled is that it completely breaks local browsing of the site, since links always point to the live anthology.

I see two options here:

Making the behaviour dependent on Hugo's "environment" variable, using .RelPermalink when in development mode and $paper.url otherwise. I don't really like this, as diverging behaviour has the potential to hide bugs.

Generating the flat structure directly with Hugo, as discussed in Should the canonical URL be the landing page? #480, instead of relying on redirects.

mjpost · 2019-08-31T15:47:45Z

hugo/layouts/papers/single.html

@@ -44,7 +44,7 @@
 {{ $bibfile := printf "/papers/%s/%s/%s.bib" (slicestr $volume_id 0 1) $volume_id $anthology_id }}
 <section id="main">
  <h2 id="title">
-    <a href="{{ $paper.url }}">{{ $paper.title_html | safeHTML }}</a>
+    <a href="{{ $paper.url }}/">{{ $paper.title_html | safeHTML }}</a>
  </h2>


The paper page has a / at the end.

mjpost · 2019-08-31T15:48:02Z

hugo/layouts/papers/single.html

      {{ end }}
      {{ $endfile := printf "/papers/%s/%s/%s.endf" (slicestr $volume_id 0 1) $volume_id $anthology_id }}
      {{ if (fileExists (printf "/data-export/%s" $endfile)) }}
-        <a class="btn btn-secondary btn-sm" href="{{ $endfile | relURL }}">EndNote</a>
+        <a class="btn btn-secondary btn-sm" href="{{ $paper.url}}.endf">EndNote</a>
      {{ end }}


Use the canonical URL prefix instead of the deeply nested prefix.

Same concern as with $bibfile above.

So do you suggest that we generate bib, endf, etc links in the YAML, and only use those if present?

This is what I ended up doing for clarity.

This is nice, but has the same issue with local browsing–the links will always point to the live server.

hugo/static/.htaccess

mjpost · 2019-08-31T15:49:44Z

hugo/static/.htaccess

-# The Paper metadata page (e.g., P17-1069/ -> papers/P/P17/P17-1069/index.html)
-RewriteRule ^([A-Za-z])([0-9][0-9])\-([0-9][0-9][0-9][0-9])\/$ papers/$1/$1$2/$1$2-$3/ [L,NC]
+# The Paper metadata page (e.g., P17-1069/ loads papers/P/P17/P17-1069/index.html)
+RewriteRule ^([A-Za-z])([0-9][0-9])\-([0-9][0-9][0-9][0-9])\/$ papers/$1/$1$2/$1$2-$3/index.html [L,NC]



This loads the index file when https://aclweb.org/anthology/P19-1002/ (with a slash) is requested.

mbollmann · 2019-09-02T10:28:37Z

Besides my individual comments, I really don't like this mixing of URL logic. URLs should either be constructed in the YAML or in the template, but right now it's a mixture of both, which seems very error-prone to me. (And in fact I believe this will break things, since we have external links as well.)

mjpost · 2019-09-04T10:12:36Z

Thanks @mbollmann, it sounds like this needs more work. What do you think about just pushing up #520 in the meantime? (I've actually had this live for some time).

mbollmann

Yes, let's make sure this functionality won't break external links, and ideally refactor it to the YAML if feasible.

mjpost · 2019-09-07T16:43:29Z

One way to do this would be for the YAML to have the following fields:

url: the paper landing page
pdf: absolute URL to the PDF, whether internal or external
endf, .xml, etc for our other formats

We then just print their values. Thoughts? I think we can use this PR as the official PR for #480.

akoehn · 2019-09-22T14:56:45Z

@mbollmann, do you have time for a review? You are more qualified, but otherwise I will have a look at it next week.

mbollmann

Some minor functional comments, but my main issue with this is how it breaks local browsing of the site, as detailed in the comments.

bin/anthology/papers.py

mbollmann · 2019-09-22T17:14:08Z

hugo/layouts/papers/list-entry.html

    {{- end -}}
    {{- $bibfile := printf "/papers/%s/%s/%s.bib" (slicestr $volume_id 0 1) $volume_id .Params.anthology_id -}}
    {{- if (fileExists (printf "/data-export/%s" $bibfile)) -}}
-    <a class="badge badge-secondary align-middle mr-1" href="{{ $bibfile | relURL }}" data-toggle="tooltip" data-placement="top" title="Export to BibTeX">
+    <a class="badge badge-secondary align-middle mr-1" href="{{ $paper.url }}.bib" data-toggle="tooltip" data-placement="top" title="Export to BibTeX">
      bib


So, the question is whether we still want a check for the existence of the file in the Hugo template.

If we do, then this current solution is suboptimal IMO since it requires us manually remembering and updating the actual local path to the file (in the $bibfile := ... line), in case it ever changes.

If we do not, we should just throw out the surrounding lines altogether.

mbollmann · 2019-09-22T17:22:14Z

hugo/layouts/papers/list-entry.html

@@ -28,7 +25,7 @@
    {{ if eq hugo.Environment "development" }}
      <span class="badge badge-light align-middle">{{ .Params.anthology_id }}</span>
    {{ end }}
-    <strong><a class="align-middle" href="{{ .RelPermalink }}">{{ $paper.title_html | safeHTML }}</a></strong>
+    <strong><a class="align-middle" href="{{ $paper.url }}/">{{ $paper.title_html | safeHTML }}</a></strong>
    <br />


The downside of how this is now handled is that it completely breaks local browsing of the site, since links always point to the live anthology.

I see two options here:

Making the behaviour dependent on Hugo's "environment" variable, using .RelPermalink when in development mode and $paper.url otherwise. I don't really like this, as diverging behaviour has the potential to hide bugs.

Generating the flat structure directly with Hugo, as discussed in Should the canonical URL be the landing page? #480, instead of relying on redirects.

hugo/layouts/papers/single.html

mbollmann · 2019-09-22T17:28:14Z

hugo/layouts/papers/single.html

      {{ end }}
      {{ $endfile := printf "/papers/%s/%s/%s.endf" (slicestr $volume_id 0 1) $volume_id $anthology_id }}
      {{ if (fileExists (printf "/data-export/%s" $endfile)) }}
-        <a class="btn btn-secondary btn-sm" href="{{ $endfile | relURL }}">EndNote</a>
+        <a class="btn btn-secondary btn-sm" href="{{ $paper.url}}.endf">EndNote</a>
      {{ end }}


This is nice, but has the same issue with local browsing–the links will always point to the live server.

mjpost · 2019-09-22T21:32:50Z

I'm not sure I like either (1) or (2) above:

It's ugly, and I agree about potentially hidden bugs; and
It's ugly to put all those files in one directory, especially the top-level directory.

What about

We adapt make serve to run a docker container that contains an Apache instance that makes use of our .htaccess file. This has the advantage that it would be more fully testing our setup. We'd also have to change the generated URLs to be relative to the local root, which is a good change to make anyway. It would be a bit of a pain for developers to setup, but maybe it's a reasonable amount of pain to expect. It could also help us with the mirroring situations.

akoehn · 2019-09-25T09:53:46Z

It's ugly to put all those files in one directory, especially the top-level directory.

Yes, we then have a lot of files in one directory. But this does not slow down anything, it is "only" a personal preference to have files sorted into directories (don't mean it insulting, I share the general preference). In fact, we lose complexity and the website might actually become faster with a flat structure.

This reduced complexity might also make it easier to reason about bugs as the html files directly show the problems and no mental step through redirects is necessary.

To not have all files in a single directory, we could instead also have a sub-directory for HTML, one for bibtex, etc.

I just fear that we add more complexity (docker containers, switches on the environment) where we could reduce it. Sure, a complex system can look beautiful, but a simple system is simple.

mbollmann · 2019-09-25T10:45:44Z

FWIW, I believe that changing the output locations of the paper pages should be as simple as adding

[permalinks]
  papers = "/:filename/"

to hugo/config.toml. The .RelPermalink variable will adjust accordingly. (Untested though.)

mjpost · 2019-09-25T12:49:43Z

One worry is that our hosting provider might not have the same modern file system. mount reports them all as "virtfs". Do you think this is possible?

What if we kept the current nested links for all generated files (bib, xml, endnote), but also added the top-level RewriteRule? That way they will both work. It's really only the .pdf links and the landing page that require only one external URL.

mjpost · 2019-09-26T15:10:44Z

Okay, but this wouldn't work for the landing page.

What if we:

Put each paper as a root-level directory (e.g., N19-1002/)
Put all generated files inside it (e.g., N19-1002/N19-1002.bib, and N19-1002/index.html for the landing page)
Use these direct local paths for all links
Also add a top-level redirect that will only work in production settings (e.g., https://www.aclweb.org/anthology/N19-1002.bib silently loads https://www.aclweb.org/anthology/N19-1022/N19-1002.bib, and the latter will redirect to the former)

This adds a bit more structure to the top-level and has direct links. It does mean that the generated files are accessible from two paths, but I don't think that's a problem for those kinds of files.

Second, does anyone have time to do this before October 1? I will own it since I am the one who announced the date, but maybe it's quicker for someone else.

akoehn · 2019-09-26T16:03:51Z

One worry is that our hosting provider might not have the same modern file system.

Ext4 has been a stable file system since 2008, that is more than ten years ago. I am sure they don't host this on FAT32 or similar ...

With your second approach, we have more structure but would probably have a similar slow-down if the server cannot handle thousands of entries in a single directory.

akoehn · 2019-09-27T16:23:46Z

Second, does anyone have time to do this before October 1?

Unfortunately not me, I am lying in my bed with a fever :-(

Do whatever you want to achieve this and ignore all my comments if it makes your life easier.

mjpost · 2019-09-27T23:03:31Z

Sorry to hear it, @akoehn! I hope you feel better soon.

I ran some tests on the server, and there is no problem with all the files in one directory. So let's go that route.

The only choice left is whether to put generated files in subdirectories or not (e.g., bib files under bib/). Unless I hear otherwise, I will just put them in the root directory. So the root will have, for example:

N19-1017/
    index.html
N19-1017.bib
N19-1017.xml
N19-1017.endf
...

knmnyn · 2019-09-28T03:47:53Z

Feel better! Hope you get well soon!

…

On Sat, Sep 28, 2019 at 12:23 AM Arne Köhn ***@***.***> wrote: Second, does anyone have time to do this before October 1? Unfortunately not me, I am lying in my bed with a fever :-( Do whatever you want to achieve this and ignore all my comments if it makes your life easier. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#513?email_source=notifications&email_token=AABU72ZNCKZP7ETL6UTZ7TLQLYXRFA5CNFSM4IMQ24I2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7ZNIBI#issuecomment-536007685>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABU72YLDPYFIVP5P5KVEKTQLYXRFANCNFSM4IMQ24IQ> .

mjpost · 2019-09-28T18:05:54Z

Last update: I've looked into this a bit, and I don't know hugo well enough to be able to implement the single directory by Monday. I'm going to suggest that we use the current proposal (which breaks local builds) until someone has a minute to fix it according our discussion here.

mbollmann · 2019-09-29T11:41:46Z

I've noticed a couple of instances where links don't point to the right location yet; for example, the metadata field citation_pdf_url which, I believe, is used for Google Scholar indexing, doesn't actually point to the PDF anymore!

I can try to fix those, but debugging the website without a working local build is substantially harder...

mjpost · 2019-09-29T13:40:58Z

How hard is it to fix the local builds? I’m happy to have everything in one directory but I don’t know hugo well enough.

mbollmann · 2019-09-29T14:37:14Z

For the pages it should be as simple as the config file change I've wrote about above, but I'd want to test it and make sure that all the links point to the correct places. The bibliography files shouldn't be too hard either, but the same caveat applies.

I might have some time later tonight, but no promises. What time exactly would you want to push this live?

mjpost · 2019-09-29T17:54:42Z

I don't think the time is important. It would be good if we could make it available sometime on the 1st, though.

mbollmann · 2019-09-29T19:00:26Z

I've added two commits that should do what we discussed, and appear to produce the right thing locally.

We could put bibliography files in a subdirectory as well (in that case I'd go with the /paper-id/paper-id.bib route) and have the link point to /paper-id.bib – that's a trivial change, but to make this work locally I think we'd have to introduce a flag that changes the URL accordingly.

mjpost · 2019-09-29T19:42:30Z

Great, I'll test this out.

mjpost · 2019-09-29T19:56:33Z

One note: in the Python, I changed the meaning of the "url" field to denote the page itself (the landing page, the canonical URL), whereas "PDF" links to the PDF. So on the paper landing page, "URL" should be a self link, instead of to the PDF.

mbollmann · 2019-09-29T20:00:37Z

It always used to link to the PDF though, so this would be a functional change.

mbollmann · 2019-09-29T20:01:56Z

Also consider pages with external links, like LREC publications, which also point to the PDF in the "URL" field:

https://www.aclweb.org/anthology/L14-1003/

mjpost · 2019-09-29T20:15:53Z

Okay, you've convinced me. I'll merge this tomorrow night!

* removed redirect rules (should have been in #513) * added nodalida to events page (closes #544)

* Removed nested links in favor of canonical variants * Flatten hierarchy of paper pages, change ANTHOLOGY_URL -> _PDF * Fix URLs that should point to PDFs * Fix volume-level URLs that should point to PDFs * Generate bibliography files in flat structure

* removed redirect rules (should have been in acl-org#513) * added nodalida to events page (closes acl-org#544)

Redirect nested paper pages to the / variant

67a9f9e

mjpost requested review from mbollmann and a team August 18, 2019 01:09

Removed nested links in favor of canonical variants

28346fc

mjpost mentioned this pull request Aug 19, 2019

dblp indexing of ACL publications #468

Open

mjpost commented Aug 31, 2019

View reviewed changes

mbollmann requested changes Sep 4, 2019

View reviewed changes

mjpost added 2 commits September 21, 2019 08:54

Merge branch 'master' into redirect

5d0eb30

moved link logic to python, updated templates, htaccess

7f3d87a

mjpost changed the title ~~Redirect nested paper pages to the / variant~~ [For merging on 30.09] landing page is canonical Sep 22, 2019

mjpost requested review from mbollmann and a team September 22, 2019 12:16

fixed typo in URL substitution

2aa1744

mbollmann requested changes Sep 22, 2019

View reviewed changes

title links to PDF

f7113d7

mbollmann added 2 commits September 29, 2019 13:42

Fix URLs that should point to PDFs

f97cf29

Fix volume-level URLs that should point to PDFs

5ec0aa1

mbollmann added 2 commits September 29, 2019 20:28

Flatten hierarchy of paper pages, change ANTHOLOGY_URL -> _PDF

6577a9c

Generate bibliography files in flat structure

f486b2c

mbollmann approved these changes Sep 29, 2019

View reviewed changes

mbollmann requested a review from a team September 29, 2019 19:17

Merge branch 'master' into redirect

fb0f84b

Merge branch 'master' into redirect

75d51d2

mjpost merged commit 19a337f into master Oct 1, 2019

mjpost deleted the redirect branch October 1, 2019 00:35

mjpost added a commit that referenced this pull request Oct 1, 2019

Hotfixes (#547)

1aa8041

* removed redirect rules (should have been in #513) * added nodalida to events page (closes #544)

mbollmann mentioned this pull request Oct 17, 2019

Make "URL" the canonical URL instead of .pdf #587

Closed

mbollmann mentioned this pull request Nov 1, 2019

Search box #165

Open

najtin pushed a commit to ir-anthology/ir-anthology that referenced this pull request Jun 9, 2021

Hotfixes (acl-org#547)

756f30f

* removed redirect rules (should have been in acl-org#513) * added nodalida to events page (closes acl-org#544)

[For merging on 30.09] landing page is canonical #513

[For merging on 30.09] landing page is canonical #513

Conversation

mjpost commented Aug 18, 2019

mjpost commented Aug 18, 2019

mjpost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjpost Sep 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbollmann commented Sep 2, 2019

mjpost commented Sep 4, 2019

mbollmann left a comment

Choose a reason for hiding this comment

mjpost commented Sep 7, 2019

akoehn commented Sep 22, 2019

mbollmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjpost commented Sep 22, 2019

akoehn commented Sep 25, 2019

mbollmann commented Sep 25, 2019

mjpost commented Sep 25, 2019

mjpost commented Sep 26, 2019

akoehn commented Sep 26, 2019

akoehn commented Sep 27, 2019

mjpost commented Sep 27, 2019 • edited Loading

knmnyn commented Sep 28, 2019 via email

mjpost commented Sep 28, 2019

mbollmann commented Sep 29, 2019

mjpost commented Sep 29, 2019

mbollmann commented Sep 29, 2019

mjpost commented Sep 29, 2019

mbollmann commented Sep 29, 2019

mjpost commented Sep 29, 2019

mjpost commented Sep 29, 2019

mbollmann commented Sep 29, 2019

mbollmann commented Sep 29, 2019

mjpost commented Sep 29, 2019

mjpost Sep 22, 2019 •

edited

Loading

mjpost commented Sep 27, 2019 •

edited

Loading