-
-
Notifications
You must be signed in to change notification settings - Fork 1
Add desktop action #31
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
Conversation
wpkelso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with including this, but I think it should use "New File" instead of "New Document" to keep consistent with the terminology established by Code
|
I mirrored the string for new documents in the app for consistency |
|
Ah. "Document" should probably be switched to "File" throughout then. I think it probably makes more sense than document, given we are just dealing with text files and not rich text documents. |
|
I dont know. In french, "document" makes the most sense for written text. I dont know how it is in other languages ill correct this here when the rest of the app is files then |
wpkelso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking so long to review this
src/Application.vala
Outdated
| public static uint created_documents = 1; | ||
| public static string data_dir_path = Environment.get_user_data_dir () + "/slate"; | ||
|
|
||
| public bool newdocument = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. I probably did try another approach but forgor to remove it
| string[] args = command_line.get_arguments (); | ||
|
|
||
| switch (args[1]) { | ||
| case "--new-document": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are your thoughts on adding a check to look for unsaved new documents so we can skip over them and always create a new blank one? Currently, we just open the first unsaved document — which might already contain something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to count the number of unsaved documents in the folder, add 1, and start from there.
Else we may not give it a proper number (Ex: New Document, overwriting current New Document)
Another hurdle would be that, what if while im editing my cool new document i want to open another unsaved document ?
We would need to expose something in the UI, which is out of scope, or close all windows and open normally. Which is pretty much what a quick desktop action was supposed to save from
|
Tbh working on Slate made me kinda want to work on a more fully featured text document for eOS. I was considering asking the author of https://github.com/manexim/typewriter to take it over and do just that, and leave Slate to do the simple Right now our attention is on porting old eOS apps to Gtk4+Wayland+Portals. I can happily leave this as draft but i dont think i can help you much more with Slate... Im sorry |
For #30
Needs testing. Looks like it work in the cases i tried, but may be shoddy.
the feature may not be needed at all since there an accel and a button. But if anyone ever want it, its here
If you want to avoid feature creep on slate, absolutely no hard feelings for dismissing this one.