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

Export selection and slight update to right-click operation #1226

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

dustinhartlyn
Copy link
Contributor

I have continued to update my fork. As noted in pull request #1224, tools stay active until canceled. A cancel operation (escape, spacebar or right-click) Will first cancel any current drawing action, if none it will unselect the selected tool. This update allows right click to unselect a tool before brining up right-click menu.

The second addition is an option to export only the currently selected lines (File->Export 2d Selection). I find this valuable because I often want to export only a portion of my design to a dxf. I know its possible to move those lines to a specific layer(s), but I think it tedious to move lines from group to group depending on what I am exporting. I find the flow much better when I can simply select what I want and export it regardless of groups.

As before, I offer these alterations only because I appreciate what others have done for this repository. Not offended if they are rejected because I intend to keep developing my fork in the ways I think best.

dustinhartlyn and others added 14 commits March 30, 2021 22:05
On windows 10 the open/save dialogue box has an minor error, and I believe I fixed it. 

When "Open" is selected from the menu, the title of the dialogue box says "SolveSpace - Save File" and the entered file name is "united". My fix correctly titles the dialoged box, and leaves the address bar blank when a file is being opened because "united" is only needed as a default name when a file being saved. 

I found that class FileDialogImplWin32 from guiwin.cpp contains two if statements for "isSaveDialog". This is redundant. I removed the first where the title was originally set, but not working. I then set the title in the second if statement and moved the 'if isEmpty'' to this section.
replaced tabs with spaces
…mouse position directly. Simplified MouseScroll in mouse.cpp to point to this function instead of altering zoom directly. Also pointed zoom commpand from keyboard and menu to ZoomToMouse so that it works avoids different behavior.
Created ZoomToMouse function in graphicswin.cpp which referances the …
Adition: Menu option (linked to spacebar) which alternates between last selected tool and mouse.
…curve in the process of being placed. It will also reselect the latest tool, but unselect the tool if no cure is in the process of being drawn.

Acomplished by using UndoUndo function which undoes the action registered after the operation is canceled, but does not add it to the redo stack so that Redo will not add the curve that was removed by the cancel operation.

CancelPending is the main function added to facilitate any trigure that cancels the current action in process. UndoUndo was altered to not store the undone action if false is passed in as an aurgument.

AlternateTool was also updated to work more efficently with CancelPending.

Last a specific command was added to the  MenuEdit function for the escape key.
Integrate continual use of tools until unselect, and cancel drawing action without placing it

First
I found it annoying to click on the same tool each time I wanted to draw a shape. My alteration reselects the tool after it is used. For example this allows the user to draw rectangles continuously after only selecting the tool once.
Second
I find it really useful to switch between the mouse and the most recently selected tool with a hotkey.

Esc still clears the selected tool, and hotkeys switch between tools as expected.
ClearPending() is used to process through tools - such as drawing the first point of a rectangle, and then placing the second point. I altered the function to store the original tool command and reactivate it when the ClearPending has reached the end of its command chain. Since all the tools funnel through this function, the one change works for all without any additional change. However I realized that it might not always be desired to reactivate a tool, so I altered the Toolbar[] array which stores all the tools. I added the 'keep_active' bool which allows authors to specify which tools should be reactivated (1) and which should be unselected after a single use (0).

To accomplish the flip-flop between the last selected tool and the mouse I added the AlternateTool() which is called by the Command::ALTERNATE_TOOL case.

Canceling a pending action will undo it and not add to redo memory
@phkahler
Copy link
Member

I'd appreciate a few things with PRs. Please provide one feature per PR so we can easily review and merge/reject them. From looking at the list I see:

  1. fix a dialog on Windows
  2. a zoom to mouse function - which probably fixes the recent breakage
  3. escape cancels the last drawing operation rather than completing it
  4. right click behavior that I don't think we want
  5. the main behavioral change in tool use

Assuming 1-3 work as expected and are PRs with 1 commit each, I'd accept them. It would then be easier to evaluate 4 and 5 independently and accept, reject, improve, or offer feedback.

Looking at "Update gui win" I see "Export selection and slight update to right-click operation" but the export selection feature doesn't even appear to be part of that commit. We try to keep a fairly clean git history, so comments should match commits. It's not clear what the right-click change does, or why its Windows specific. It's things like this that drive the need to do one PR per feature (even if the feature is more than one commit).

@dustinhartlyn
Copy link
Contributor Author

I believe you have already accepted the pull request for the window fix and the fix for the zoom. I am still new to Git so I am sorry if my pull request was messy. The referenced pull request contains the cancel-operation behavior and to switch to tools being used until canceled. I can look into separating the cancel behavior out but right now I am getting into the task of adding line offsets / contours so it might not be immediate.

@phkahler
Copy link
Member

@dustinhartlyn Oh yes, i accepted a previous PR. Since you're new to git I have some suggested things to learn. Of course the first is to learn vim, since git commands are sometimes going to dump you into vi to edit some text. I learned vim by running "vim tutor" every day for a week and learning the first few sections. (or maybe you're not new to vi).

Assuming you forked solvespace on github and then cloned it to your local repository, you should have the ability to reference "origin" as your github repo. You'll also want to setup "source" to point at the solvespace/solvespace repositiory. I don't recall how to do that, but once done you can checkout your master branch and type "git pull upstream master" to pull all the changes from our master then "git push origin master" to push them up to your github master.

If your changes are on a branch and you've updated your local master to match ours, you can switch to your branch and do "git rebase master" which should bring your branch history up to date and not show it differing from master by so many commits. Then you can push it to the branch on your origin and the PR will update as well.

Squashing commits is done with "git rebase -i HEAD~X" where X is the number of commits you want to rearrange. You'll need to read some instructions on rebasing and squashing.

I hope some of this helps. We can use more contributors to Solvespace ;-)

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

2 participants