https://bugs.dragonflybsd.org/https://bugs.dragonflybsd.org/favicon.ico?16293952082009-03-23T08:28:44ZDragonFlyBSD bugtrackerDragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63352009-03-23T08:28:44Zjoelkp
<ul></ul><p>I did mess something up, namely parameter handling of new option in<br />main() - the kludge around getopt was incomplete, and so, while the<br />use examples in the previous mail would work, reordering them like<br />this wouldn't:</p>
<p>undo -d -3 -2</p>
<p>Instead, that became equivalent to:</p>
<p>undo -d -32</p>
<p>This fixed in new patch.</p> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63402009-03-25T23:55:08Zdillon
<ul></ul><p>:> This patch does not change the usage notice nor documentation, so this<br />:> remains to do. (and perhaps stylistic or other changes if the code is<br />:> not up to your standards or I messed up somewhere)<br />:<br />:I did mess something up, namely parameter handling of new option in<br />:main() - the kludge around getopt was incomplete, and so, while the<br />:use examples in the previous mail would work, reordering them like<br />:this wouldn't:<br />:<br />:undo <del>d -3 -2<br />:<br />:Instead, that became equivalent to:<br />:<br />:undo -d -32<br />:<br />:This fixed in new patch.<br />:<br />:-</del> <br />: - Joel K. Pettersson</p>
<pre><code>Hmm.</code></pre>
<pre><code>Ok, the -t ts1/ts2 fixes look good.</code></pre>
<pre><code>The 0-9 handling needs some help. A lot of help. It's just too messy.<br /> I think it needs to be handled either by giving it its own option <br /> (instead of trying to parse 0-9 in getopt), or by integrating the index<br /> selection with -t by detecting that the TID values are not in 0x form.</code></pre>
<pre><code>My preference is to simply make -t accept non 0x form transaction ids<br /> as indices instead of transaction ids.</code></pre>
<pre><code>-Matt</code></pre> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63462009-03-28T07:15:24Zjoelkp
<ul></ul><p>I'm not too surprised.</p>
<p>I guess trying to use immediate number options with multiple<br />characters is simply not possible to get clean as opposed to merely<br />working when getopt is used for options. While slightly more verbose<br />in usage, the code for using a standard option with argument is rather<br />much nicer, yes.</p>
<p>Should I send such a patch? Alternatively, if you copy the main()<br />function from before the feature patch, move the initiation of the<br />"flags" variable to before the option-reading loop, then such code can<br />simply be inserted for case 't'.</p>
<p>With a simple test for index numbers, though it may be a bit<br />quick-and-dirty, here's such code:<br /> case 't':<br /> if (ts1.tid == 0 &&<br /> !(flags & UNDO_FLAG_SETTID1)) {<br /> ts1.tid = parse_delta_time(optarg);<br /> if (optarg<sup><a href="#fn0">0</a></sup> >= '0' && optarg<sup><a href="#fn0">0</a></sup> <= '9' &&<br /> optarg<sup><a href="#fn1">1</a></sup> != 'x')<br /> flags |= UNDO_FLAG_SETTID1;<br /> } else if (ts2.tid == 0 &&<br /> !(flags & UNDO_FLAG_SETTID2)) {<br /> ts2.tid = parse_delta_time(optarg);<br /> if (optarg<sup><a href="#fn0">0</a></sup> >= '0' && optarg<sup><a href="#fn0">0</a></sup> <= '9' &&<br /> optarg<sup><a href="#fn1">1</a></sup> != 'x')<br /> flags |= UNDO_FLAG_SETTID2;<br /> } else<br /> usage();<br /> break;</p>
<p>Where the code for detecting non-TIDs may be replaced, if needed.</p> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63482009-03-28T13:43:00Zdillon
<ul></ul><p>:orm.<br />:I guess trying to use immediate number options with multiple<br />:characters is simply not possible to get clean as opposed to merely<br />:working when getopt is used for options. While slightly more verbose<br />:in usage, the code for using a standard option with argument is rather<br />:much nicer, yes.<br />:<br />:...<br />:Should I send such a patch? Alternatively, if you copy the main()<br />:function from before the feature patch, move the initiation of the<br />:"flags" variable to before the option-reading loop, then such code can<br />:simply be inserted for case 't'.</p>
<pre><code>Send a complete patch w/ everything, using the new scheme, and I'll<br /> commit it.</code></pre>
<pre><code>I'm not sure the logic you posted will properly sequence ts1 and<br /> ts2. I suggest just tracking the sequencing separately instead<br /> of using ts1.tid and the flags variable.</code></pre>
<pre><code>Then for updating the flags just pass additional arguments<br /> to parse_delta_time() and let parse_delta_time() make the<br /> distinction and adjust the flags e.g.:</code></pre>
<pre><code>case 't':<br /> ++count_t;<br /> if (count_t == 1)<br /> ts1.tid = parse_delta_time(optarg, &flags, UNDO_FLAG_SETTID1);<br /> else<br /> ts2.tid = parse_delta_time(optarg, &flags, UNDO_FLAG_SETTID2);<br /> else<br /> usage();<br /> break;<br /> ...</code></pre>
<pre><code>Or something like that.</code></pre>
<pre><code>-Matt<br /> Matthew Dillon <br /> &lt;<a class="email" href="mailto:dillon@backplane.com">dillon@backplane.com</a>&gt;</code></pre> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63572009-03-29T10:40:02Zjoelkp
<ul></ul><p>Here goes. Also includes the changes in the earlier bugfix patch (for<br />making ts2 be used at all after being set in main()) from the mail<br />before this whole thread, so if committed, both (fix and feature) are<br />resolved.</p>
<p>Changed accordingly.</p> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63602009-03-29T14:49:29Zdillon
<ul></ul><p>:Here goes. Also includes the changes in the earlier bugfix patch (for<br />:making ts2 be used at all after being set in main()) from the mail<br />:before this whole thread, so if committed, both (fix and feature) are<br />:resolved.<br />:<br />: - Joel K. Pettersson</p>
<pre><code>It looks very nice. I have committed it along with some additional<br /> augmentation of the documentation. I also adjusted the code so if<br /> two -t options are given it automatically defaults to DIFF mode (-d),<br /> unless overridden.</code></pre>
<pre><code>-Matt</code></pre> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=63612009-03-29T21:50:57Zjoelkp
<ul></ul><p>On another note, a second feature addition for the index parameter<br />occurred to me, which became a very simple matter with the new code -<br />making a negative index do something, namely go forward from the<br />oldest revision.</p>
<p>While not as significant from a usability point as the previous<br />addition, perhaps the best point of it is a quick way of accessing the<br />oldest, for example diffing it with the newest, as in:</p>
<p>undo -dt-0 filename</p>
<p>This patch, unlike that of the previous addition, has documentation<br />updates, so should be a quick matter if of interest.</p>
<p>- Joel K. Pettersson</p> DragonFlyBSD - Bug #1319: another undo patch: a feature request and implementation, should it be of interesthttps://bugs.dragonflybsd.org/issues/1319?journal_id=64492009-04-23T14:52:26Zcorecode
<ul></ul><p>committed in 965778c8e5068e125ea2aa029359fdf88d1e5dfc</p>