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

Path.absname, absolute paths for insert date #1847

Closed
wants to merge 1 commit into from

Conversation

introt
Copy link
Collaborator

@introt introt commented Dec 30, 2021

Complements Path.relname and Path.basename. In addition, replaced
instances of "':' + link.name" with this new property and changed
links created by the "Insert Date and Time" dialog absolute.

Complements Path.relname and Path.basename. In addition, replaced
instances of "':' + link.name" with this new property and changed
links created by the "Insert Date and Time" dialog absolute.
introt referenced this pull request in Osndok/zim-desktop-wiki Dec 30, 2021
@introt
Copy link
Collaborator Author

introt commented Dec 30, 2021

This addition could also be useful for implementing a "only use absolute links" option (see #492 (comment))

@introt

This comment has been minimized.

@introt introt closed this Dec 31, 2021
introt added a commit to introt/zim-desktop-wiki that referenced this pull request Dec 31, 2021
This patch builds on the broken Path.absname one (zim-desktop-wiki#1847) and thus
isn't recommended for daily consumption.

See zim-desktop-wiki#492 for further discussion.
@introt introt deleted the absolute-date-links branch December 31, 2021 16:09
@jaap-karssenberg
Copy link
Member

The attribute path.name is the full name of the path. The syntax ":" + path.name creates wiki syntax for an absolute link. Probably the most clean way to handle this would be to use a HRef object rather than a path object ?

@introt
Copy link
Collaborator Author

introt commented Jan 6, 2022

Looking back, my problem was creating Paths from wiki syntax for relative links, against the instructions in the documentation (Never construct a path directly from user input).

Probably the most clean way to handle this would be to use a HRef object rather than a path object ?

Which part? I tried[0] handling InsertDateDialog with a HRef just now, ended up with HRef(HREF_REL_ABSOLUTE, self.link.name).to_wiki_link() which seems excessive for prepending a : - self.link being a Path instead of a HRef is a tad bit confusing.

I can't say I've wholly cracked the distinction between Path and HRef. Why not have Path represent absolute paths with eg. from_relative_link(source: Path, target: str) and to_wiki_link(relative_source: Path=None) to serve the HRef needs? Being half-rhetorical here, probs has something to do with either maintainability/performance/something I-can't-quite-articulate regarding relative/floating links or semantics. Though semantically I also find using rel in the context of "href" for location-relation instead of resource-relation confusing, but that's likely due to me not grasping the more abstract concepts and relating the terminology to html - I better stop here before I start questioning why we don't have a Link class.

  1. https://github.com/introt/zim-desktop-wiki/tree/href-date

@jaap-karssenberg
Copy link
Member

jaap-karssenberg commented Jan 6, 2022 via email

introt added a commit to introt/zim-desktop-wiki that referenced this pull request Jan 7, 2022
* zim.notebook.index.links
  - comment -> docstring
  - type annotated IndexLink
* zim.notebook.index.pages
  - type annotated PagesView
* zim.notebook.page
  - type annotated Path, HRef, shortest_unique_names
  - improved HRef documentation based on zim-desktop-wiki#1847
@introt
Copy link
Collaborator Author

introt commented Jan 7, 2022

Thanks, that clears it up a lot!

PageName is (at least in this context) a lot more descriptive - and is used in some parameter names already. Are you open to a PR for this?

If you are sure you need an absolute link, you can indeed just call ":" +
path.name, however it may not be robust in some far future where an
alternative syntax is supported.

":" + path.name is already used in a few places. Would the following addition to HRef make sense to you? Pagename shouldn't need another round of validation or resolving, and this allows the future-proof creation of absolute links without importing HREF_REL_ABSOLUTE.

@classmethod
def pagename_to_wiki_link(klass, pagename):
    '''Returns href for C{pagename} as text for absolute wiki link.'''
    #return ':' + pagename.name
    return klass(HREF_REL_ABSOLUTE, pagename.name).to_wiki_link()

I've opened #1853 for some related documentation improvements.

introt added a commit to introt/zim-desktop-wiki that referenced this pull request Jan 7, 2022
* zim.notebook.index.links
  - comment -> docstring
  - type annotated IndexLink
* zim.notebook.index.pages
  - type annotated PagesView
* zim.notebook.page
  - type annotated Path, HRef, shortest_unique_names
  - improved HRef documentation based on zim-desktop-wiki#1847
@jaap-karssenberg
Copy link
Member

jaap-karssenberg commented Jan 8, 2022 via email

jaap-karssenberg pushed a commit that referenced this pull request Jan 25, 2022
* zim.notebook.index.links
  - comment -> docstring
  - type annotated IndexLink
* zim.notebook.index.pages
  - type annotated PagesView
* zim.notebook.page
  - type annotated Path, HRef, shortest_unique_names
  - improved HRef documentation based on #1847
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.

None yet

2 participants