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: document tracking issue when path of document changed #178

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

huddeldaddel
Copy link
Contributor

Proposed Changes

This change removes the tracking of documents being renamed via document.onDidRename.
It seems that is actually not required (anymore) since the previous document gets removed and then added again. I check that for all use cases I could identify:

  • Creating a new file / saving it
  • Opening a file / saving to different location

Closes #176

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@CLAassistant
Copy link

CLAassistant commented Sep 17, 2024

CLA assistant check
All committers have signed the CLA.

@huddeldaddel
Copy link
Contributor Author

So far I was not able to come up with a unit test that can save a document. Saving the current document to a new destination would be part of the required user interaction to test for this change. The available commands (that I found) do not accept a URL destination - instead they require user interaction via file dialog.

@barmac
Copy link
Member

barmac commented Sep 20, 2024

Thanks for the PR. Perhaps if the change cannot be tested via API, we will manually confirm the fix.

@barmac
Copy link
Member

barmac commented Sep 20, 2024

In the meantime, please adjust the commit message to adhere to conventional commits: https://www.conventionalcommits.org/
We use that for semantic releases.

@huddeldaddel huddeldaddel changed the title Removed redundant tracking of document renamings fix: document tracking issue when path of document changed Sep 20, 2024
Copy link
Member

@barmac barmac left a comment

Choose a reason for hiding this comment

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

It works fine in manual tests. Let's try this out.

@barmac
Copy link
Member

barmac commented Sep 23, 2024

I noticed you updated the PR title but not the commit message. I will do that in the merge.
image

@barmac barmac merged commit 998f30b into bpmn-io:main Sep 23, 2024
5 checks passed
@github-actions github-actions bot mentioned this pull request Sep 23, 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.

"Save as..." causes unexpected error: document already exists
3 participants