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

Generalize citar--shorten-names + add display transform function for full names #808

Closed

Conversation

torfjelde
Copy link

Generalizes citar--shorten-names + implements rendering of full names as discussed in #805.

Replaces #807 as I did not follow the contribution guidelines in #807.

Add new display transform function for full names.

Generalize existing `citar--shorten-names` to `citar--render-names`
to allow code-sharing between completion rendering of family names
and full names.

Refs: emacs-citar#805
@torfjelde
Copy link
Author

torfjelde commented Oct 23, 2023

@bdarcus are there any tests for this stuff? Didn't see anything obvious from a quick glance.

@bdarcus
Copy link
Contributor

bdarcus commented Oct 23, 2023

are there any tests for this stuff?

Do you mean in this project, or more generally?

If the former, this is an example (see #780) where we should probably add some.

@torfjelde
Copy link
Author

For the project, and specifically of the display transform functions:)

But aight; should I just make a new ert-deftest for the short- and full-name methods? Are there an existing list of author names that I could maybe draw from as test-cases, or should I add some?

@bdarcus
Copy link
Contributor

bdarcus commented Oct 23, 2023

Are there an existing list of author names that I could maybe draw from as test-cases, or should I add some?

There isn't, but I can give you an example of a key type that I included when developing the existing function; a corporate (organizational) name:

World Bank

That shouldn't be shortened.

Now that I think about it, you should be aware of the discussion here, in part because OP has some test names, but also because it's about refactoring that function:

#532

@bdarcus
Copy link
Contributor

bdarcus commented Feb 28, 2024

So what do we want to do with this, given @aikrahguzar's alternative?

#805 (comment)

@bdarcus
Copy link
Contributor

bdarcus commented May 13, 2024

So what do we want to do with this, given @aikrahguzar's alternative?

#805 (comment)

I'm going to close this, and we can consider @aikrahguzar's alernative.

@bdarcus bdarcus closed this May 13, 2024
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.

2 participants