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

fix+refactor: link handling #292

Merged
merged 8 commits into from
Apr 3, 2024

Conversation

kokoISnoTarget
Copy link
Contributor

@kokoISnoTarget kokoISnoTarget commented Mar 30, 2024

This fixes part of #277.
Now we set the current directory of inlyne when opening a file, meaning that relative path work by default.
Which means that some of the logic in link handling can be removed.

Now we set the current directory to the directory of the file so that relative link can be opened by open::that, also added an error message when open::that fails.
@kokoISnoTarget kokoISnoTarget marked this pull request as draft March 30, 2024 23:52
@kokoISnoTarget kokoISnoTarget marked this pull request as ready for review March 31, 2024 00:55
@CosmicHorrorDev CosmicHorrorDev added the C-refactor Category: Reworking an existing feature label Apr 1, 2024
@CosmicHorrorDev CosmicHorrorDev self-requested a review April 1, 2024 02:18
@CosmicHorrorDev
Copy link
Collaborator

(edited the original message because GH was picking up "fixes #xxx" as closing words and it seems like you can't just un-link it)

Copy link
Collaborator

@CosmicHorrorDev CosmicHorrorDev left a comment

Choose a reason for hiding this comment

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

Everything looks great other than the internal conditional logic to get tests working, but I think there are a few options we have on how to tackle that

If you want I can always make changes and push as well

src/history.rs Outdated Show resolved Hide resolved
src/history.rs Outdated Show resolved Hide resolved
@kokoISnoTarget
Copy link
Contributor Author

That would be great.

@CosmicHorrorDev
Copy link
Collaborator

That should clear up everything, so I'm fine with merging now as long as it looks good to you @kokoISnoTarget

@kokoISnoTarget
Copy link
Contributor Author

Yea seems good.

@CosmicHorrorDev CosmicHorrorDev merged commit 2a77a7d into Inlyne-Project:main Apr 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-refactor Category: Reworking an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants