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

feat(file): support non-path files #775

Closed
wants to merge 4 commits into from
Closed

feat(file): support non-path files #775

wants to merge 4 commits into from

Conversation

bdarcus
Copy link
Contributor

@bdarcus bdarcus commented Apr 7, 2023

  • add citar-file-scheme-skip to define URI file schemes to pass on without validating or normalizing
  • citar-file--find-files-in-dirs: use citar-file-scheme-skip

Close: #685


@dlukes - this turned out to be trickier than I thought, because the existing code was all assuming regular file paths.

EDIT: need to look now at citar-has-files, as the indicators aren't correctly matching returned file paths in citar-open, and opening files in general aren't working correctly.

citar-file.el Outdated Show resolved Hide resolved
citar-file.el Outdated Show resolved Hide resolved
* add citar-file-scheme-skip to define URI file schemes to pass on
  without validating or normalizing
* citar-file--find-files-in-dirs: use citar-file-scheme-skip

Close: #685
We don't need a function for this; just store it as a global variable.
citar-file.el Show resolved Hide resolved
@bdarcus bdarcus marked this pull request as draft April 10, 2023 13:42
@bdarcus bdarcus marked this pull request as ready for review April 10, 2023 13:47
@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 11, 2023

@dlukes: I think I'm going to put this aside, as once I make progress in one area, I end up breaking other things.

Any suggestions on easiest/best to support #775 @roshanshariff?

Basically, we need to refactor the file code so it doesn't assume only ever standard file paths, with the initial case zotero links.

@roshanshariff
Copy link
Collaborator

I haven't been following this development very much, but I wonder if it's better to just bypass the citar-file functionality completely. In other words, make a separate citar-zotero module that handles everything to do with Zotero attachments, including showing the indicators and handling the "open" action. That way citar-file can stay focused on actual files, since most of its functionality is not relevant to Zotero attachments anyway.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 11, 2023

I wonder if it's better to just bypass the citar-file functionality completely. In other words, make a separate citar-zotero module that handles everything to do with Zotero attachments, including showing the indicators and handling the "open" action.

So zotero resources then would be a separate type of related resource, in a separate group from "files"?

And therefore also a separate command from citar-open-files?

They are still files, though, they just use a different mechanism to find them, and are opened in a different viewer, effectively.

I did a few weeks ago merge a change that allow one to set citar-open-entry to open in Zotero, rather than a bib or json file. That felt right to me.

But this has gotten more complicated, as I mentioned.

And a separate module opens up other questions, which I'm loathe to tackle, at least anytime soon.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 15, 2023

I think I'm going to close this for now.

@bdarcus bdarcus closed this Apr 15, 2023
@dlukes
Copy link

dlukes commented Apr 18, 2023

Apologies, the whole family's been ill on and off for the past week or so, and I just can't afford to invest much time into this right now, especially as the Citar codebase is mostly unfamiliar to me. Again, sorry!

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 18, 2023 via email

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.

Open PDF in Zotero
3 participants