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

Add truncate as a BibTex key modifier #6427

Merged
merged 38 commits into from
May 13, 2020

Conversation

k3KAW8Pnf7mkmdSMPHz27
Copy link
Member

@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 commented May 5, 2020

Fixes #3915. Adds a truncate modifier to the available BibTex key modifiers. For "A Title" the bracketed pattern [title:truncate3] produces the result "A T".

  • The syntax is truncateN ([title:truncate5]) to mimic the pattern of authN etc.

Remaining

  • See if layered expressions are reasonable to implement, e.g., [[auth][title]:truncate10]
  • Update changelog
  • Documentation

Things I am not sure about

  • Should truncate be a formatter(org.jabref/logic/formatter)? Currently, formatters does not accept arguments
  • The interaction between BibTex key generation and truncate might lead to unexpected results as whitespaces are removed after truncation. Unless this is considered obvious, it should perhaps be mentioned in the documentation of truncate. E.g., for "A Title", [title:truncate3] will lead to the bibtexkey "AT"
  • Should this be included in the changelog?
  • There were a BracketedPatternTest in org.jabref/logic/util/. Is there a correct location for the BracketedPatternTest?

Others

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@Siedlerchr
Copy link
Member

  1. Changelog, yes please
  2. Ideally it would be a formatter, maybe we need to adapt the interface to allow arguments.
  3. Whitespace: At least it should be documented
  4. BrackedPatternTest, yes would be nice if you could integrate your tests there, as they test with a bibtex entry and multiple formatters

Otherwise looks already good as starting point

@k3KAW8Pnf7mkmdSMPHz27
Copy link
Member Author

2. Ideally it would be a formatter, maybe we need to adapt the interface to allow arguments.

I missed how RegexFormatter is implemented, I'll just mimic that and it shouldn't be a problem.

@k3KAW8Pnf7mkmdSMPHz27
Copy link
Member Author

Updates to documentation.

@k3KAW8Pnf7mkmdSMPHz27
Copy link
Member Author

In a nutshell.

  1. truncateN has been added as a Formatter.
  2. Directory names are truncated to a length of 255 characters using a utility function for filenames.
  • I don't think I should change the parser, there is a large risk I will create more issues than I solve
  • It might be possible to shorten the code in java/org/jabref/logic/bibtexkeypattern/BracketedPattern.java quite a bit if larger parts of the code is turned into Formatters and Formatters can accept arguments (either in the constructor or in the format method, both are currently being used).

@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 changed the title [WIP] Add truncate as a BibTex key modifier Add truncate as a BibTex key modifier May 8, 2020
@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 marked this pull request as draft May 11, 2020 13:01
@k3KAW8Pnf7mkmdSMPHz27
Copy link
Member Author

k3KAW8Pnf7mkmdSMPHz27 commented May 11, 2020

When I try to import a file from an URL using a pattern that creates a java.nio.file.FileSystemException. The directory cannot be created but the file is still downloaded. Should I try to fix that as part of this pull request as well?

If a file cannot be downloaded to the location specified by a file pattern it makes more sense (to me) that the file isn't downloaded at all.

@Siedlerchr
Copy link
Member

your changes look so far, is this ready then?

@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 marked this pull request as ready for review May 12, 2020 13:25
@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 changed the title [WIP] Add truncate as a BibTex key modifier Add truncate as a BibTex key modifier May 12, 2020
# Conflicts:
#	src/test/java/org/jabref/logic/bibtexkeypattern/BracketedPatternTest.java
@k3KAW8Pnf7mkmdSMPHz27
Copy link
Member Author

Yes. The last conflicts should be resolved now.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 12, 2020
@k3KAW8Pnf7mkmdSMPHz27
Copy link
Member Author

The last change (change of test names) can be removed but it seems more consistent with the guidelines?

@Siedlerchr
Copy link
Member

For me it's fine. For external contributors we have the rule that at least two developers should review a PR. So when a second dev gives his okay, it's ready for merge!

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

The code looks good to me, so I'll merge now. Thanks for your contribution - we look forward to your next PR!

@tobiasdiez tobiasdiez merged commit 16d4938 into JabRef:master May 13, 2020
Siedlerchr added a commit that referenced this pull request May 16, 2020
* upstream/master: (50 commits)
  Keep group pane size when resizing window (#6180) (#6423)
  Changelog: Fix missing citation for biblatex-mla
  Update AUTHORS
  Check duplicate DOI (#6333)
  Fix missing citation for biblatex-mla
  Change EasyBind dependency (#6480)
  Add testing of latest dev version as mandatory
  Squashed 'src/main/resources/csl-styles/' changes from 5dad23d..586e0b8
  Fix libre office connection and other progress dialogs (#6478)
  Fix clear year and month field when converting to biblatex (#6434)
  Add truncate as a BibTex key modifier (#6427)
  Add new authors (not all - they need more work)
  Remove empty line
  Add simple Unit Tests for #6207 (#6240)
  Enforce LeftCurly rule (#6452)
  Implement task progress indicator (and dialog) in the toolbar (#6443)
  Consider empty brackets
  Changelog update
  Added a test
  Fixed brackets in regular expressions
  ...
koppor pushed a commit that referenced this pull request Mar 1, 2023
e9fd2027de Add Medicine Publishing Styles (#6434)
cae128f35f Create Bristol University Press (#6356)
74b4af3b82 Create internet-archaeology.csl (#6357)
ee7ece480b Add Bio-Protocol style (#6429)
9a455efcee Create archives-of-medical-research.csl (#6415)
e91aba46fc Remove some bursa-uludag styles (#6423)
03f3962657 Update offa.csl (#6428)
95dc9b9f5a Update journal-of-neolithic-archaeology.csl (#6427)
a4e6c7f477 Update the-university-of-winchester-harvard.csl (#6374)
c0bf10647a add manuscript formatting to ASA (#6387)
3a673a564a Update universite-de-sherbrooke-histoire.csl (#6392)
0c48c7289e Update chemistry-education-research-and-practice.csl (#6397)
51f718a7b9 Update journal-of-endodontics.csl (#6409)
51e419051f Update presses-universitaires-de-rennes.csl (#6413)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: e9fd2027de4e2355f3244ac662960467e225774d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Offer truncation of file names during file renaming
3 participants