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

Package refactoring #74

Merged
merged 81 commits into from
Jan 24, 2020
Merged

Package refactoring #74

merged 81 commits into from
Jan 24, 2020

Conversation

DavidLuptak
Copy link
Collaborator

I previously made some refactoring and didn't merged, so I create this PR only for your peer review, otherwise I could merge it by myself.

Merge conflict arises probably by bumped version, I hope..

Copy link
Collaborator

@moewew moewew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few observation about your changes and also added a few more general remarks that might be interesting to look into.

biblatex-iso690.tex Outdated Show resolved Hide resolved
biblatex-iso690.tex Outdated Show resolved Hide resolved
biblatex-iso690.tex Outdated Show resolved Hide resolved
biblatex-iso690.tex Outdated Show resolved Hide resolved
biblatex-iso690.tex Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
mybib.bib Outdated Show resolved Hide resolved
mybib.bib Outdated Show resolved Hide resolved
…ge options and availability of the fields; regression introduced in a12fc21 and #64
iso.bbx Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
iso.bbx Outdated Show resolved Hide resolved
@DavidLuptak
Copy link
Collaborator Author

What about merging this PR as it covered already a lot of changes?

@michal-h21 michal-h21 merged commit e322d11 into master Jan 24, 2020
\DeclareFieldFormat{isbn}{%
%\renewcommand*{\do}[1]{\mkbibacro{ISBN}\addspace#1\adddot\space}%
\mkbibacro{ISBN}\addspace#1}%
\def\do##1{\stdidentifierspunct\mkbibacro{ISBN}\space##1}%
Copy link
Collaborator

@moewew moewew Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that definition \stdidentifierspunct also applies before the first ISBN. Given the description says "between more standard identifiers" that was a bit unexpected to me and might not actually be desired.

\documentclass[english]{article}
\usepackage[T1]{fontenc}
\usepackage[utf8]{inputenc}
\usepackage{babel}
\usepackage{csquotes}

\usepackage[style=iso-authoryear, thesisinfoinnotes=false, backend=biber]{biblatex}

\renewcommand\stdidentifierspunct{XXX}

\addbibresource{biblatex-examples.bib}

\begin{document}
\autocite{cms}
\printbibliography
\end{document}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I will dig in the details of this commit a bit:

  • the original \do command was not working at all with \renewcommand syntax (I supposed it should)
  • I used \def clause, which I remember you are not fond of it, but only this works for me in this case
  • I relied on the \stdidentifierspunct will not be printed if it contains punctuation symbol(s), because it follows some other punctuation from the previous block, but it's true that someone might use sth like or the other ISBN as a "punctuation"
  • if \stdidentifierspunct is at the end of \do command, it also applies if there is only one ISBN, which is also not desired
  • I couldn't figure out how to easily check if there is more than one number in the list
  • now I realized we can change type of isbn from the field to list entry type and then check with \ifmoreitems for multiple ISBNs, can't we?

I would really like to do it the right way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • It shouldn't really matter if we use \def or \renewcommand
    \renewcommand*{\do}[1]{\stdidentifierspunct\mkbibacro{ISBN}\space##1}%
    works just as well.
  • While I usually prefer not to use \def, I think it is OK with \do. The etoolbox documentation uses \renewcommand*{\do}, but all instances but one in the biblatex core say \def\do
  • Yeah, you'd usually want to know the number of items in advance and it would be nice to have a running counter as well. I don't know how to do that without looping over the field first just for counting and looping over it for a second time for the actual typesetting. In this case it is enough to cheat a bit
    \documentclass[british]{article}
    \usepackage[T1]{fontenc}
    \usepackage[utf8]{inputenc}
    \usepackage{babel}
    \usepackage{csquotes}
    
    \usepackage[style=iso-authoryear, backend=biber]{biblatex}
    
    \renewcommand\stdidentifierspunct{XXX}
    
    \DeclareFieldFormat{isbn}{%
      \def\do##1{\mkbibacro{ISBN}\space##1%
        \def\do####1{\stdidentifierspunct\mkbibacro{ISBN}\space####1}}%
      \docsvfield{isbn}%
    }
    
    \begin{filecontents}[force]{\jobname.bib}
    @book{appleby,
      author  = {Humphrey Appleby},
      title   = {On the Importance of the Civil Service},
      date    = {1980},
      isbn    = {1-234,456-7},
    }
    \end{filecontents}
    \addbibresource{\jobname.bib}
    
    
    \begin{document}
    \cite{appleby}
    \printbibliography
    \end{document}
  • Yes, a list would be the best solution if you want multiple ISBNs.
    \documentclass[british]{article}
    \usepackage[T1]{fontenc}
    \usepackage[utf8]{inputenc}
    \usepackage{babel}
    \usepackage{csquotes}
    
    \begin{filecontents}[force]{iso-authoryear.dbx}
    \ProvidesFile{iso-authoryear.dbx}[2017/04/25 v0.3.2 biblatex data model extension]
    \RequireBiber[3]
    \DeclareDatamodelFields[type=list,datatype=name]{supervisor}
    \DeclareDatamodelEntryfields[thesis]{supervisor}
    
    
    \DeclareDatamodelFields[type=list, datatype=literal]{isbn}
    \end{filecontents}
    
    \usepackage[style=iso-authoryear, backend=biber]{biblatex}
    
    \newbibmacro*{list:stdidentifierspunct}[1]{%
      \ifnumgreater{\value{listcount}}{\value{liststart}}
        {\stdidentifierspunct}
        {}}
    
    \DeclareListFormat{isbn}{%
      \usebibmacro{list:stdidentifierspunct}%
      \mkbibacro{ISBN}\space#1%
      \usebibmacro{list:andothers}}
    
    
    \newbibmacro*{identifier}{%
      \iftoggle{bbx:isbn}
        {\printfield{isan}%
         \newunit
         \printlist{isbn}%
         \newunit
         \printfield{ismn}%
         \newunit
         \printfield{isrn}%
         \newunit
         \printfield{issn}%
         \newunit
         \printfield{iswc}%
         \newunit}
        {}%
    }
    
    \begin{filecontents}[force]{\jobname.bib}
    @book{appleby,
      author  = {Humphrey Appleby},
      title   = {On the Importance of the Civil Service},
      date    = {1980},
      isbn    = {1-234 and 456-7},
    }
    \end{filecontents}
    \addbibresource{\jobname.bib}
    
    
    \begin{document}
    \cite{appleby}
    \printbibliography
    \end{document}
    Would work, but uses the usual list format with and and not with commas, which might be a bit of a backwards compatibility issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed that cheat version of \DeclareFieldFormat{isbn} into master, since as the standard says, there shouldn't be more ISBNs for the one specific entry, so use this only as the last fallback and do not temp users to adjust bib entries to have more ISBNs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants