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: empty folder's disappearing when copying #1719

Closed
wants to merge 1 commit into from

Conversation

Tiger3018
Copy link
Contributor

@Tiger3018 Tiger3018 commented Mar 31, 2024

TODO:

  • portfolio_message omit copy event of the empty folder

Related: #1452, #1633

@Tiger3018
Copy link
Contributor Author

Notes:

For folder layers, DocumentNode.inputs seems to be [Child_node/Value, Sibling_node]. It's not well documented yet.

@0HyperCube
Copy link
Member

That is correct in the current main branch @Tiger3018, however @Keavon is currently reversing the ordering of those inputs (I'm unsure why) in a branch in someone's fork. For documentation on what the inputs are, you can look at the implementation (here the ConstructLayerNode struct) and also the document_node_types.rs file has a string name for the inputs to a node.

@Tiger3018
Copy link
Contributor Author

For highlighting and group issues, it only appeared in Just a Potted Cactus and Valley of Spires example files. New document and two other example files didn't affected. Maybe a document version issue?

Mark this PR ready for review.

@Tiger3018 Tiger3018 marked this pull request as ready for review April 1, 2024 02:50
TODO: highlight of the copied empty folder will be gone when its parent changes.
@Keavon
Copy link
Member

Keavon commented Apr 1, 2024

!build

Copy link

github-actions bot commented Apr 1, 2024

📦 Build Complete for 30366ba
https://2696d198.graphite.pages.dev

@Keavon Keavon marked this pull request as draft April 4, 2024 04:03
@Tiger3018 Tiger3018 changed the title Fix: portfolio_message (base+document) regarding grouping/folders Fix: portfolio_message omit copy event of the empty folder, leading to their disappearing Apr 4, 2024
@Tiger3018
Copy link
Contributor Author

This PR is not related to discovered issues formerly, and can be merged

@Tiger3018 Tiger3018 marked this pull request as ready for review April 5, 2024 08:27
@Keavon
Copy link
Member

Keavon commented Apr 6, 2024

I'm sorry, I actually still can't figure out what this PR addresses. What was the original issue or todo item? Can you please edit the PR description to mention only that?

@Tiger3018 Tiger3018 changed the title Fix: portfolio_message omit copy event of the empty folder, leading to their disappearing Fix: empty folder's disappearing when copying Apr 6, 2024
@Tiger3018
Copy link
Contributor Author

@Keavon I have removed the portfolio_message to clarifying just the phenomenon in the PR title, and have removed the mention of new issue in the description.

@Keavon Keavon force-pushed the master branch 2 times, most recently from 7bf7f92 to 5f4960d Compare April 25, 2024 01:06
@Keavon
Copy link
Member

Keavon commented Apr 29, 2024

I'm still not sure precisely what this PR is about but I think it's already solved by the changes made in #1712 so I will close this as it's redundant.

@Keavon Keavon closed this Apr 29, 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.

None yet

3 participants