Am 19.04.10 07:21, schrieb Derek Scherger: > 2010/4/15 Derek Scherger <de...@echologic.com> >> 2010/4/14 Stéphane Gimenez <d...@gim.name> >> >>> First remark: mtn status ask for my password and it's rather >>> inconvenient (I'm not using ssh agent). Same for mtn commit just after >>> it's invocation and prior to the real commit. >> >> Hrm.. I hadn't noticed that but I'll take a look. > > This should be fixed in c66c53da6285693772f76e3f7cfa3b99a34f8b04 but I'm not > sure if the change I've made there is a good one or not. > > Thomas and Tim, can you please have a look at this revision and see if you > have a better approach? I don't see any obvious way of getting the signing > key without attempting to decrypt it.
I guess we can't do much better right now. We have lots of places where
static functions fiddle / evaluate global state and change other global
state / calculate some value and I'd consider this bad practice, but
hey, there is no way to fix that without rewriting lots of code, right?
In the meantime what I would really really want to get rid of are
boolean function arguments - so maybe you could start and make the code
a little easier to read by passing a more describable enum value...?
>>> About this, I find 'ChangeSet: xxxx' really confusing, one could
>>> think of 'xxxx' as an id for the changeset. The good old 'Changes against
>>> parent xxxx' looks better to me.
>>>
>>
> I tend to agree here and I've reverted it to use the "Changes against
> parent..." however I'm now wondering whether displaying the branch names
> associated with each parent in this section might be useful which makes me
> wonder again about using Parent: ... and Branch: headers here.
This might surely make sense, but where do we stop? If we start listing
cert values of *parents* we might start listing the parents of these
parents and their certs and... endless loop :)
No, I think the parent revision id is enough, really. Everything else
should be part of that revision's log output.
>>> Since you want to know about what users think... See below for what
>>> I'd like to see in my editor. I think it's pretty self explanatory
>>> and compatible with most of the ideas you came up with.
>>>
>>
> Here's what I have now:
>
> $ mtn commit
> ######################################################
> Enter a description of this change following the Changelog line.
> The values of Author, Date and Branch may be modified as required.
> Any other modifications will cause the commit to fail.
>
> *** REMOVE THIS LINE TO CANCEL THE COMMIT ***
> ----------------------------------------------------------------------
> Revision: c66c53da6285693772f76e3f7cfa3b99a34f8b04
> Parent: f016f1e3e91e181e1fee2320ad537d99ce236d7d
> Author: de...@echologic.com
> Date: Sun Apr 18 10:59:06 PM 2010
> Branch: net.venge.monotone.experiment.changelog-editor
>
> Changelog:
>
> avoid requiring key passphrase for status and before editing commit message
>
> * cmd_ws_commit.cc (status,commit): call get_user_key with new
> don't-cache-key
> flag; pass key_store object into complete_key_identity so that keys that
> don't exist in the database are still available
>
> * keys.{hh,cc} (get_user_key): add new "cache" boolean controlling whether
> check_and_save_chosen_key is called to decrypt the private key
>
> * tests/wrong_options_override_workspace_options/__driver__.lua: revert
> changes
> made on this branch that are no longer necessary; test now matches that
> on
> net.venge.monotone
>
> ----------------------------------------------------------------------
> Changes against parent f016f1e3e91e181e1fee2320ad537d99ce236d7d
>
> patched cmd_ws_commit.cc
> patched keys.cc
> patched keys.hh
> patched tests/wrong_options_override_workspace_options/__driver__.lua
> ######################################################
>
> Which includes a --- separator line after the Changelog section in the
> commit editor. I've left the Revision: and Parent: lines between the ---
> separator lines alone. I'm assuming that people won't have any reason to
> edit these as they won't have any sensible values to put in them so they
> aren't likely to touch them.
Ok, I got my separator lines - fair 'nuff...
> $ mtn status
> ----------------------------------------------------------------------
> Revision: aa124b3ff5c488a0aeba8754821d00a374c61440
> Parent: c66c53da6285693772f76e3f7cfa3b99a34f8b04
> Author: de...@echologic.com
> Date: Sun Apr 18 11:12:45 PM 2010
> Branch: net.venge.monotone.experiment.changelog-editor.foo
>
> ----------------------------------------------------------------------
> This revision will create a new branch.
> Old Branch: net.venge.monotone.experiment.changelog-editor
> New Branch: net.venge.monotone.experiment.changelog-editor.foo
>
> Changes against parent c66c53da6285693772f76e3f7cfa3b99a34f8b04
>
> no changes
Now this is something which I'd slightly change - I'd love to see this
information in the editable area and the hint that the branch changes
above the editable area - because its important and might otherwise get
easily forgotten, so something like:
$ mtn status
ATTENTION: This revision will create a new branch.
----------------------------------------------------------------------
Revision: aa124b3ff5c488a0aeba8754821d00a374c61440
Parent: c66c53da6285693772f76e3f7cfa3b99a34f8b04
Author: de...@echologic.com
Date: Sun Apr 18 11:12:45 PM 2010
Old branch: net.venge.monotone.experiment.changelog-editor
Branch: net.venge.monotone.experiment.changelog-editor.foo
----------------------------------------------------------------------
Changes against parent c66c53da6285693772f76e3f7cfa3b99a34f8b04
no changes
and for commit the header could look like so
$ mtn commit
Enter a description of this change following the Changelog line.
The values of Author, Date and Branch may be modified as required.
Any other modifications will cause the commit to fail.
*** REMOVE THIS LINE TO CANCEL THE COMMIT ***
ATTENTION: This revision will create a new branch.
----------------------------------------------------------------------
>>> Also, it would be nice if the editor was invoked with option '+13' to
>>> position the cursor just after the ChangeLog line. It seems to work
>>> with vim, emacs and nano.
>>
>>
> You can set EDITOR='emacsclient +15' to get what you want but this is
> probably not what you want EDITOR set to in general. We'd need to do some
> more work in the edit_comment hook to get this right. This would be nice to
> do but I don't think it's critical for getting this branch finished.
Instead of giving the editor a "jump point" (which could be error-prone
f.e. if you think about i18n) I'd try to decrease the verbosity of the
entry line mainly by removing the warning in line three:
$ mtn commit
Enter a description of this change after the Changelog line.
Author, Date and Branch may be modified as required.
*** REMOVE THIS LINE TO CANCEL THE COMMIT ***
----------------------------------------------------------------------
And of course I'd remove Revision: and Parent: from the editable area to
make it even less verbose, but we've had this discussion before and as I
said I'm happy already that my separators made it in ;)
> I'm not in any particular hurry to merge this but I think it's pretty much
> ready. Are there any fundamental objections at this point? Do we want to
> have it in for 0.48?
I think we definitely want to have it for 0.48. And I'm also voting for
not discussing this useful feature to death (because then it won't get
included at all) - so I'm shutting up now :)
Thomas.
--
GPG-Key 0x160D1092 | tommyd3...@jabber.ccc.de | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Monotone-devel mailing list Monotone-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/monotone-devel
