+23
Completed

Changes view: allow editing file

vseticka martin 7 years ago updated by Thomas Singer 3 years ago 17

When I review changes that I'm about to commit I often find that I made a typo. I would like to make some adjustments in SmartGit right away to avoid navigating to the file in my IDE.

I reckon with "review window" you mean the "Changes" view. Currently, it changes its content on selection change in the "Files" view. What do you expect the "Changes" view to do after you have changed something and select a different file? IMHO a selection change should not show a confirmation dialog.

Yes, "Changes" window. I don't know how to change it in my original post.


It seems convenient not to show a confirmation dialog as you say.

OK, let's discuss this further. Normally, a change in the selection of the Files view should cause to show the file changes in the Changes view. If it became changed by the user, it should visually noticable disconnect from the selection change in the Files view and offer a Save or Discard option. If either is selected, it would listen again for selection changes in the Files view, right?

Should the Changes view be editable directly or should it be made editable by an explicit action, e.g., by a toolbar button click?

> If either is selected, it would listen again for selection changes in the Files view, right?


Yes


> Should the Changes view be editable directly or should it be made editable by an explicit action, e.g., by a toolbar button click?

Directly editable sounds better to me.

I also would like to edit the displayed content (from index or working tree, obviously not HEAD) of the Changes view directly. Opposed to having to open the index editor, 'tediously' visually search / scroll to the place I want to edit, close the editor again. After all, isn't one important purpose of the Changes view to check/verify the modifications I made to the working tree and/or index? Well, checking means the possibility to spot errors. And if I do spot an error, I'd like to fix it right away.


So effectively the Changes view would no longer be a read only text view, but a simple editor. Actually two, one for each side (unless the underlying file is from HEAD). I imagine that as soon as the text has been edited, it visually indicates, as Thomas said, that the displayed contained is modified relative to the underlying file (working tree or index). You cannot 'navigate away', e.g. by selecting another element in the Files view, until you explicitly save or discard your modifications. I.e. each side of the changes view would need two additional buttons: save and discard. If you try to 'navigate away' anyway, a Dialog offers to save/discard/cancel.

This would be a very bad user experience. A selection change never must cause a dialog to occur. I also consider a dialog on a focus change as bad user experience.

I completely agree in general. I envisioned that in general no dialog would occur after a selection change.


I imagine it like this: When the user modifies a text in the Changes view, she really explicitly wants the current 'read only viewer' (which can be closed at any time) to become a simple 'text editor'. When this 'text editor' is closed while having unsaved changes relative to the underlying file, e.g. due to selection change in the Files view, IMHO it is appropriate to show a modal dialog asking what to do with these unsaved changes. The purpose of this simple text editor is to make tiny modifications, typically fixing small errors spotted while inspecting changes using the current Changes view. When you want to do more serious editing, you should use another editor.

Completed

We've tried to implement this feature, but found it not easy and intuitive to use. Instead, the next 17.1 preview build will allow to open the Index editor at the same place where the Changes view is, so you can edit there.

I'm not sure whether I should select now Declined or Completed - I use the latter. ;)

Thanks. I'm looking forward to testing this. :)

You already can do that with 17.1 preview from http://www.syntevo.com/smartgit/preview and by invoking Help | Check for Latest Build. If the Changes view is focused/selected and you invoke the Index Editor, it will occur at the same place at the Changes view.

Was the feature "Edit in changes view" implemented?

The biggest disadvantage in invoking "Index Editor" is the extra step we have to take in order to do a quick edit.

+1
What would be the alternative? Let's say, you could edit directly, should it automatically disconnect from the table selection or discard any change if a refresh occurs or the user selects a different file? Or should the modification immediately be written to disk? Neither would be nice, so invoking the Index Editor seems to be the simplest and easiest to understand concept.

Hi Thomas,

IMHO, it should just edit. When user switches to a different file the program should ask for saving. refresh (don’t know how that works) - should ... I don’t know :)

Changes should be saved to disk when I press ctrl+s.



Problem with Index Editor is that .... it is not intuitive.... For me the easiest solution is to edit the directly. No extra steps (like opening another window).

    +1

    We won't implement a modal dialog on a selection change because we consider this very bad user interface design.

    Such feature should be a configurable setting. Turned off by default, but available for those who want to work in a faster way. Something like "Show Index Editor by default".

    I honestly don't understand why this would work any different than how it would work in Smart SVN, which if recall allowed the in place edit.

    I believe the current default is not optimal. When you click on the text area, you expect to be able to type and edit the text. That is what the user naturally expects. It's a major drag that you can't. I have updated the keyboard shortcuts so that ctrl+e brings up the full editor, but that's not the quick edit (and I don't believe it's possible to assign that to shortcut key). Switching between windows is also a highly taxing and user unfriendly experience, especially when you are trying to go thru many files making quick changes in each. 

    I believe the following behavior would be preferred:

    - Allow editing in place, without an any extra clicks to start editing, period.

    -  When editing in place, there are two context buttons that appears in the section inbetween the file list and the file viewer. The buttons are in effect "save edit" or "cancel edit".

    - The ability to accept changes from the the head or working tree into the index, or move changes from the index to the working tree, works just like in the full editor, depending on what view is activated. 

    - In other words, things behave identically to the full text editor when all three views are displayed. Even though only two views will be visible in the quick edit, any changes in the invisible view will just be parked there, unsaved. The user can switch between the two views and the unsaved changes will be visible in either the index or working tree as applicable.
    - This state will persist until the user does something:

      - Save

      - Cancel

      - Clicks on a different file (resolution provided below)

    - Clicking elsewhere in SmartGit, minimzing, etc., has no effect on this workflow. Only when a new file viewing is to replace the existing one, via the user's explicitly choice to click on a different file, will trigger the below workflow.

    - The full text editor is also still an option just like it is now, and will behave the same. If there are local unsaved changes in either the index or working tree when the full editor is invoked, they will be brought forth into the full editor, remaining in an unsaved state within the full editor.

    - Once in the full editor, the user attempting to leave the full editor with unsaved changes will pop up the confirmation modal just like it does now. The user will not be allowed to bring the unsaved changes back into the quick edit. 

    - In effect, this means that opening up the full editor is a way for the user to delay their making a choice on saving the content or discarding it while still being allowed to view a different file -- they can spawn as many full editor windows as they want, so if they start making a quick edit, and are unsure on what they want to do in terms of saving or discarding their work, but want to view a different file, they can do so with a simple keyboard shortcut to bring up their current quick editing changes into the full editor.

    From here, the main question is how to deal with the user leaving the view / clicking on a different file within the quick edit:

    - In effect, clicking on a different file behaves the same as "closing" the file in the full text editor. It's natural for users to expect to be prompted what to do when that happens and there are unsaved changes. But if you really don't want a modal, and believe that clicking on a different file isn't the same as an explicit close action by the user, you can do the following:

    Three modes:

    1 (Default): If user clicks on a different file, the current changes are auto-saved and the next file editor is brought up. The user will be able to see the dirty status reflected immediately in the file browser.

    2 (override option from preferences): When clicking on a different file and the changes are unsaved, the dialog modal will be brought up stating that clicking on a different file will close the current quick editor, and ask the user what they want to do. The dialog can have four radio options:

      * Save all

      * Save working tree changes only

      * Save index changes only

      * Discard all

    - In addition, there is a checkbox that appears beneath the radio choices that says "Automatically perform my choice as the default action (can be changed in preferences).

    3) (override option next to the above): Like #1 except that all the changes are discarded instead of saved.

    To be very user friendly, I would recommend that on first time this workflow is triggered on a fresh install, that #2 is actually the default behavior. That will give uses the chance to immediately tune to their liking and prevent any further modals if it's not what they prefer.

    > I honestly don't understand why this would work any different than how

    > it would work in Smart SVN, which if recall allowed the in place edit.

    No, it never did.