Move formatted paste to LaTeX Utilities, add api to get graphics path #1478
Conversation
|
I am against this PR as I explained in the previous PR. I do not want to continue the discussion as I have explained enough. I think this is up to the authors, @James-Yu @jlelong, now. |
|
I may need a bit of time to go through it. Please remind me if I did not show up in a few days. |
|
@James-Yu *cough |
|
Thanks @James-Yu I'll go through all of those either tonight or tomorrow :) |
|
@James-Yu let me know if that's better now, and if you'd like any more changes. |
|
@James-Yu *cough |
|
As @tamuratak, I am still not convinced by the Paste from clipboard function. This is true that this feature has been requested in other LaTeX extensions but it is not clear at that it has been implemented.
Not clear that pasting from clipboard has indeed been implemented but rather only pasting a file path.
But not implemented
It does not seem to have been integrated into AUCTeX. Anyway, if you believe pasting from clipboard is important, it goes beyond LaTeX and there are indeed several vscode extensions to achieve this. In particular, it looks like https://github.com/mushanshitiancai/vscode-paste-image could do it almost out of the box. |
|
@jlelong Yea, I have mixed feelings on this too; particularly given that https://github.com/mushanshitiancai/vscode-paste-image exists. Ultimately what made me think it's worth going in this direction is that it fits with my view of this extension as a "one stop shop" for the functionality you'd want with LaTeX documents.
Re: AUCTeX for it being integrated you need to skip to https://lists.gnu.org/archive/html/auctex/2010-12/msg00015.html where I saw "I integrated it into auctex/tex.el and it is working nicely" and then the discussion continues in further threads. At least I assume this means it was integrated into AUCTeX 🤷♂️. |
|
@tecosaur stop such an argument. With your argument, we must include every function requested by users. Nonsense. We must decide which one to integrate with LaTeX Workshop. |
|
I usually see adding functionality requested by users to be a good thing 😛 but yes — definitely can be a slippery slope in some cases. Anyway, I don't think there's much point in me discussing this further so I'll just wait untill James decides to weigh in. |
|
Aiming at building the All-In-One extension for LaTeX may sound a promising idea but it may turn into a trap because it will lead the extension to grow far too much. We must always weigh the benefit for most users of introducing a feature versus the maintainability issues and potential bugs. Concerning this PR, I do believe that it would better fit directly into https://github.com/mushanshitiancai/vscode-paste-image as making the LaTeX support would only be a small add-on. Modularity is always good practice in software design. Before going any further, I would suggest @tecosaur to get in touch with the maintainers of https://github.com/mushanshitiancai/vscode-paste-image. |
|
@jlelong I think that sounds like a good way of handling this. Hence, this is my revised plan
// almost-code
if (clipboard.text === '') {
try {
vscodeExecuteCommand('vscode-paste-image.paste')
} catch (e) {
vscodeShowMessage('Install vscode-paste-image to be able to paste images')
}
} |
|
Addendum. The one bit I wouldn't be sure how to do would be passing the reference for |
|
While I am interested in seeing this feature, I totally agree with @jlelong on modularity and maintainability. Let's see if the other extension would incorporate the main logic so that we only need to have a minimal adapter. |
|
I have created mushanshitiancai/vscode-paste-image#50 and I'll report back when something's happening 😃 |
|
Ok. It's now been five days and I haven't seen any sign of a response. Looking at other open issues, the last few haven't got any response either (Going back to early May). Hmmm, I'm not sure how to proceed from here. @James-Yu any thoughts? |
|
@tecosaur make your own extension. |
|
Done. This now just contains an adaptor + some formatting changes that slipped in. |
|
Why do we need this change on LaTeX Workshop's side? You misunderstand James' idea #1544 (comment). LaTeX Utilities uses some API exposed by LaTeX Workshop. Not the opposite. If changes on LaTeX Workshop's side are necessary on every new feature by LaTeX Utilities, the maintainability of LaTeX Workshop does not get better. |
|
@tamuratak Other than the API revealing on this extension's side, changes are not required for this feature. The feature works with just LaTeX Utilities using the "Paste an Image File" command. I do not see this as indicative of how other feature will be either, take the "Live Reformatting" — that is also implemented in LaTeX Utilities and requires no change this side. However, it makes logical sense that when you paste an image path the same thing happens as with CSVs etc. and if you have an image in the clipboard "formattedPaste" would logically be the command to insert it into the document, and to have the user remember yet another shortcut seems wholly unnecessary. I also fail to see any real maintinence cost induced by effectively adding four lines twice. if (this.extension.utilities) {
this.extension.utilities.exports.pasteImage()
} else {
vscode.window.showInformationMessage('Install LaTeX Utilities to paste images')
}I must say, to me it seems a bit ridiculous to be concerned about the 'maintinence cost' of that. In James's comment he talkes about modularising features. This has been done. I see now reason why we can't add a very small number of simple lines to make things work better though. |
|
When another person with a fancy idea "Paste X" appears, they will want another change on LaTeX Workshop's side. You have to discuss with each other which "Paste X/Image" to prioritize in LaTeX Workshop's side. That is a kind of increase in the maintenance cost of LaTeX Workshop. We can avoid such a thing with an appropriate API. Please think about what kind of API exposed by LaTeX Workshop will solve your problem. |
|
Thank you for explaining that @tamuratak. I was thinking of it as just this single feature.
Then the best place for the "formattedPaste" feature itself woud be in What do you think? |
|
I would vote yes for moving all formatted paste features to utilities. That makes it more clear. |
|
Ok. I think that should resolve all of this rather nicely. I'll move it tonight. |
tecosaur
changed the title
|
Besides this, we may also think of what existing features may be better re-positioned in LaTeX Utilities. To name one, the word counter feature is way too isolated and may be good as a module in LU. |
|
To me (regarding which features should be where) I think the first question that should be asked is the approach. To me, there are three main ways we can go about labelling issues as Workshop or Utilities.
|
|
I am implicitly taking the third approach. Will have more PRs to LU in a few days. |
|
I think I a new issue should be used for the discussions, I can't imagine that this is the end. Let's move to #1558 |
Note: this started off as image pasting, to see what happend read the block of comments at the end. I'll leave the original PR text below
Now introducing, the ability to directly paste images into LaTeX files
Summary
cmd+v, just pastes normally unless blank (in which case it tries an image)Background
To me this is a "Yes! Finally, this is fantastic!" type feature, however as seen by how heated #1452 became obviously not everyone shares my enthusiasm.
Justification
Simplification of the image inclusion process
Old Process
New Process
Demand / Support in other editors
Maintainability
Structure of new feature
When pasting an image there are seven main methods called:
pasteImage
Just get's selection text and calls some of the other main functions.
As long as the other functions work this will to.
Needs maintaining if: other class functions change
loadImageConfig
Bog standard reading config variables from vscode settings and then applying
replacePathVariablesNeeds maintaining if: vscode api introduced drastic breaking changes
replacePathVariables
Just performs a bunch of text replacements.
this.extension.completer.input.graphicsPathNeeds maintaining if:
this.extension.completer.input.graphicsPathchangesgetImagePath
Uses selected text or config + user to determine image path.
Needs maintaining if:
pathintroduces drastic breaking changessaveAndPaste
Calls
ensureImgDirExistswhich just usesfse, then either runsfsCopy/fsRenameorsaveClipboardImageToFileAndGetPath.fs/fsesaveClipboardImageToFileAndGetPathNeeds maintaining if:
fs/fseintroduces drastic breaking changessaveClipboardImageToFileAndGetPath
Uses
spawnfrom childprocess, calls short os-specific scripts.Scripts are short and simple, should be quite stable.
Needs maintaining if: childprocess depreciates
spawnor clipboard interaction method with os drastically changes (unlikely)renderImagePaste
Uses config from
loadImageConfigthen does text replacement.Needs maintaining if: text replacement config scheme changes
Maintainability conclusion
Shouldn't require much.