As you may know much effort has been made[1] to implement inline edit feature in Gerrit,that means to edit files directly in browser in cm3 editor with syntax highlighting.
Ground work:* Provide RevisionEditInserter & RevisionEditReader.
REST API:* Extend detail change endpoint to retrieve revision edits (new option or per default?).* PutContent endpoint: change a resource inside existing revision edit or create a new revision edit* GetContent endpoint: retrieve a resource from revision edit or from arevision if there is no revision edit for this revision and user (already there?).* Publish revision edit: promote it to be a regular patch set.* Discard revision edit: delete it.
Am Freitag, 13. Dezember 2013 13:12:54 UTC+1 schrieb David Ostrovsky:
Am Mittwoch, 11. Dezember 2013 17:19:03 UTC+1 schrieb David Ostrovsky:As you may know much effort has been made[1] to implement inline edit feature in Gerrit,that means to edit files directly in browser in cm3 editor with syntax highlighting.
[...]
Ground work:* Provide RevisionEditInserter & RevisionEditReader.
Done in [1].
REST API:* Extend detail change endpoint to retrieve revision edits (new option or per default?).* PutContent endpoint: change a resource inside existing revision edit or create a new revision edit* GetContent endpoint: retrieve a resource from revision edit or from arevision if there is no revision edit for this revision and user (already there?).* Publish revision edit: promote it to be a regular patch set.* Discard revision edit: delete it.
REST endpoints can be implemented now and can be considered as easy hack tasks.Done in [1]. Here is the first screen shot: revision edit in CS2 [2].
TODO:* make CS2 revision edit aware
SBS2 integration
> Given it is already big series i wonder if we can review/polish/merge it
> as is and do CM3 integration later?
>
I think this is a good idea. Otherwise you'll just be rebasing and
reuploading the whole series again and again. Merging it as it is will
allow others to contribute enhancements and fixes. We saw this with the
initial implementation of the new change screen.
I've checked out the series and done some quick tests locally. It all
seems to work as expected and looks good so far. There are a few nits
and glitches (list below) but as mentioned above I think it's reasonable
to merge as is and fix those in follow-up changes.
- There are a couple of warnings in Eclipse after checking out the
series. I've added inline comments on the relevant parts on the code
review.
- In "Edit Mode", after opening a file in side-by-side view by clicking
on the filename, and then clicking the "Up to change" icon, the change
screen goes back in Review Mode. It might be better to make the mode
persistent.
- The "Save" button has the tool tip "Create new patch set with updated
commit message".
- After making changes in the file, then pressing "Reload", the "Save"
button is still enabled.
- Upon publishing an edit and creating a new patch set, the review
comment "Published new edits" is added. It would be better if this were
more consistent with the existing messages that are posted when doing
some action in the UI. For example on creating patch set 2 by editing
the commit message, the message is "Patch Set 2: Commit message was
updated".
- In Edit Mode, it doesn't make sense to be able to select the "Diff
against" revision option.
- To edit the commit message while in Edit Mode, the user must still use
the "Edit Message" button on the commit message box. It would be better
if there were also an edit icon beside the commit message in the file list.
- When adding a new file in Edit Mode, the pop up dialog does not have
any labels on the input fields.
- After adding a new file, the change screen goes back to "Review Mode".
- Since we can add files, should we also be able to remove files from
the change? If so, should it completely remove the file, or only undo
the changes that were added?
- Similarly, should we be able to rename files?
- To edit the commit message while in Edit Mode, the user must still use
the "Edit Message" button on the commit message box. It would be better
if there were also an edit icon beside the commit message in the file list.
Missing feature, and can be done later. There are some other corner cases,which we probably want to disable too, in edit mode.- When adding a new file in Edit Mode, the pop up dialog does not have
any labels on the input fields.
Missing feature.- After adding a new file, the change screen goes back to "Review Mode".
Agreed, we should find a way to preserve the state.Note: adding new file amends the revision edit and reloadthe change screen.- Since we can add files, should we also be able to remove files from
the change? If so, should it completely remove the file, or only undo
the changes that were added?- Similarly, should we be able to rename files?
Missing features.We should have them all: Remove the file from repository and remove it from the change.Why not to put four icons in front of path: [EDIT], [REMOVE from REPO], [REMOVE from CHANGE], [RENAME].Renaming can be even done in place: the row can be switched into inline edit field and after changing the name and clicking enter,the file is renamed.But as you said, let's do that later.
--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
- There are a couple of warnings in Eclipse after checking out the
series. I've added inline comments on the relevant parts on the code
review.
- The "Save" button has the tool tip "Create new patch set with updated
commit message".
- After making changes in the file, then pressing "Reload", the "Save"
button is still enabled.
- Upon publishing an edit and creating a new patch set, the review
comment "Published new edits" is added. It would be better if this were
more consistent with the existing messages that are posted when doing
some action in the UI. For example on creating patch set 2 by editing
the commit message, the message is "Patch Set 2: Commit message was
updated".
- In Edit Mode, it doesn't make sense to be able to select the "Diff
against" revision option.
- When adding a new file in Edit Mode, the pop up dialog does not have
any labels on the input fields.
- Since we can add files, should we also be able to remove files from
the change? If so, should it completely remove the file, or only undo
the changes that were added?
about the code style, want to know does it apply "gerrit/tools/GoogleFormat.xml" to edit part automatically?
Thanks David, got it.
> “…enable content type specific syntax highlighting mode. “
This is cool. Today I find GitLab also can create new file and update file online (from UI) , but without syntax highlighting.
--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
---
You received this message because you are subscribed to a topic in the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/repo-discuss/20ge_59v3WI/unsubscribe.
To unsubscribe from this group and all its topics, send an email to repo-discuss...@googlegroups.com.
On Friday, 20 December 2013 07:49:56 UTC+9, David Ostrovsky wrote:
Am Donnerstag, 19. Dezember 2013 07:13:54 UTC+1 schrieb David Pursehouse:[...]
Looks like there have been a few rebases and reworks on this series since December.What's the status now? Is it ready?
Even if it's not perfect I think we should merge the series (as long as there's nothing seriously wrong with it) and then add fixes as needed. It would be really good to get this feature in 2.9.
I just tested the series and it looks really nice.
There a a few nits I found (see also comments on the add file changeset):
* The add-button is located a bit unfortunatly.
* It is possible to start editing a merged change, you get the change-closed error only when you try to publish it.
Hi,
with the new changes it looks much better to me.
One think, were I am a bit unsure: The edit mode is not persistent (after doing an edit, you are back to
review mode).
On one hand this is can be annoying, if you e.g want touch many files, on the other hand it
has the advantage that it discourages too many (big) inline edit attempts. Larger changes should in my opinion
be done in a local editing environment were you can test your changes locally as well. But hat can definitly get
changed later, if desired.
Yeah, the first change to the predecessor inline-edit-1 series, that this series is based on,was committed by Dave B. during London's Hackathon on May 8. 1013 [1], so ages ago ;-)
One question: Do you think it is OK to go with plain text editor with the initial implementation of inlineedit feature, or would you expect that Codemirror integration with syntax highlighting should be alreadyimplemented in the very first version?
--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
I've rebased the series on the latest master branch and resolved conflict caused by the change that I submitted out-of-sequence earlier this week.
It looks like [1] is the latest change that is complete; the successor of that has TODO in the commit message. I've done some quick smoke tests based on [1] and it's looking very good.
It would be great if we can get this finalised before making 2.11-rc0.
I've rebased the series on the latest master branch and resolved conflict caused by the change that I submitted out-of-sequence earlier this week.It looks like [1] is the latest change that is complete; the successor of that has TODO in the commit message.