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

Implement file duplication #1972

Merged
merged 6 commits into from Jan 12, 2023
Merged

Conversation

alvra
Copy link
Contributor

@alvra alvra commented Jan 12, 2023

This PR implements duplicating files in the fs tree context menu.

@panekj
Copy link
Collaborator

panekj commented Jan 12, 2023

I don't think adding duplication is a proper thing to do, I'd rather see Copy/Paste/Cut actions like in standard file explorers

@alvra
Copy link
Contributor Author

alvra commented Jan 12, 2023

What would be the difference between file duplication and copy? Do you mean copy to clipboard?

@panekj
Copy link
Collaborator

panekj commented Jan 12, 2023

What would be the difference between file duplication and copy?

Duplication implements very specific action which can be replaced by clipboard actions

Do you mean copy to clipboard?

Yes

@dzhou121
Copy link
Collaborator

I personally feel it's a good addition to copy/paste/cut, because I often do copy and paste which is essentially duplicate.

@panekj
Copy link
Collaborator

panekj commented Jan 12, 2023

I personally feel it's a good addition to copy/paste/cut, because I often do copy and paste which is essentially duplicate.

It's not bad per se, but I would like to avoid adding it to mouse context menu. As we develop we will be adding more actions which could be more useful and cluttering context menu with too many actions feels wrong.

@alvra
Copy link
Contributor Author

alvra commented Jan 12, 2023

True, duplication is more specific than copy+paste. Personally I use duplicate much more often and having it as a single option is helpful to me. New files often share code with its siblings so I probably use duplicate more even than "new empty file".

@dzhou121
Copy link
Collaborator

I personally feel it's a good addition to copy/paste/cut, because I often do copy and paste which is essentially duplicate.

It's not bad per se, but I would like to avoid adding it to mouse context menu. As we develop we will be adding more actions which could be more useful and cluttering context menu with too many actions feels wrong.

I don't feel it would be too much with one more menu item in the copy/paste/cut section(in the future).

@panekj
Copy link
Collaborator

panekj commented Jan 12, 2023

New files often share code with its siblings so I probably use duplicate more even than "new empty file".

We should probably hide New ... when targeting files like VSCode and show it on directories/empty space only
image
image

@dzhou121
Copy link
Collaborator

New files often share code with its siblings so I probably use duplicate more even than "new empty file".

We should probably hide New ... when targeting files like VSCode and show it on directories/empty space only image image

Yeah in a separate PR maybe.

@dzhou121 dzhou121 closed this Jan 12, 2023
@dzhou121 dzhou121 reopened this Jan 12, 2023
@panekj
Copy link
Collaborator

panekj commented Jan 12, 2023

I personally feel it's a good addition to copy/paste/cut, because I often do copy and paste which is essentially duplicate.

It's not bad per se, but I would like to avoid adding it to mouse context menu. As we develop we will be adding more actions which could be more useful and cluttering context menu with too many actions feels wrong.

I don't feel it would be too much with one more menu item in the copy/paste/cut section(in the future).

If we're not going to implement custom context actions from plugins, then yes, it should stay fine but that's quite a limitation IMO

lapce-data/src/explorer.rs Outdated Show resolved Hide resolved
lapce-data/src/command.rs Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2023

Codecov Report

Merging #1972 (94e6af1) into master (acf1de5) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master   #1972      +/-   ##
=========================================
- Coverage    8.95%   8.93%   -0.03%     
=========================================
  Files         133     133              
  Lines       57597   57769     +172     
=========================================
+ Hits         5160    5161       +1     
- Misses      52437   52608     +171     
Impacted Files Coverage Δ
lapce-data/src/command.rs 0.00% <ø> (ø)
lapce-data/src/explorer.rs 0.00% <0.00%> (ø)
lapce-proxy/src/dispatch.rs 0.00% <0.00%> (ø)
lapce-rpc/src/proxy.rs 0.00% <0.00%> (ø)
lapce-ui/src/explorer.rs 0.00% <0.00%> (ø)
lapce-ui/src/tab.rs 0.00% <0.00%> (ø)
lapce-data/src/keypress/keypress.rs 73.23% <0.00%> (+0.20%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dzhou121
Copy link
Collaborator

Can you please resolve the CHANGELOG.md conflict?

@panekj panekj merged commit 7e67729 into lapce:master Jan 12, 2023
@panekj panekj added this to the Next release milestone Jan 12, 2023
@alvra alvra deleted the feature-duplicate-file branch January 12, 2023 19:27
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

4 participants