Patchwork D8403: rust-chg: indent process_message() to prepare mass rewrite to futures-0.3

login
register
mail settings
Submitter phabricator
Date April 11, 2020, 11:24 a.m.
Message ID <differential-rev-PHID-DREV-wcaw3rf7zvyc2e2v2lcv-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46068/
State Superseded
Headers show

Comments

phabricator - April 11, 2020, 11:24 a.m.
yuja created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I'll start upgrading the codebase to modern async/await-based implementation,
  which cannot be done incrementally. This is the last non-breaking patch to
  prepare for the rewrite.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8403

AFFECTED FILES
  rust/chg/src/runcommand.rs

CHANGE DETAILS




To: yuja, #hg-reviewers
Cc: mercurial-devel
phabricator - April 14, 2020, 9:11 a.m.
Alphare added inline comments.
Alphare accepted this revision.

INLINE COMMENTS

> runcommand.rs:143
>  {
> -    match msg {
> -        ChannelMessage::Data(b'r', data) => {
> -            let code = message::parse_result_code(data)?;
> -            Ok(AsyncS::Ready((client, handler, code)))
> -        }
> -        ChannelMessage::Data(..) => {
> -            // just ignores data sent to optional channel
> -            let msg_loop = MessageLoop::resume(client);
> -            Ok(AsyncS::PollAgain(CommandState::Running(msg_loop, handler)))
> -        }
> -        ChannelMessage::InputRequest(..) | ChannelMessage::LineRequest(..) => Err(io::Error::new(
> -            io::ErrorKind::InvalidData,
> -            "unsupported request",
> -        )),
> -        ChannelMessage::SystemRequest(data) => {
> -            let (cmd_type, cmd_spec) = message::parse_command_spec(data)?;
> -            match cmd_type {
> -                CommandType::Pager => {
> -                    let fut = handler.spawn_pager(cmd_spec).into_future();
> -                    Ok(AsyncS::PollAgain(CommandState::SpawningPager(client, fut)))
> -                }
> -                CommandType::System => {
> -                    let fut = handler.run_system(cmd_spec).into_future();
> -                    Ok(AsyncS::PollAgain(CommandState::WaitingSystem(client, fut)))
> +    {
> +        match msg {

Not sure I understand the extra bracket here.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8403/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8403

To: yuja, #hg-reviewers, Alphare
Cc: Alphare, mercurial-devel
Yuya Nishihara - April 14, 2020, 12:07 p.m.
> > +    {
> > +        match msg {
> 
> Not sure I understand the extra bracket here.

It's just for rustfmt to indent one more depth.
phabricator - April 14, 2020, 12:09 p.m.
yuja added a comment.


  >> +    {
  >> +        match msg {
  >
  > Not sure I understand the extra bracket here.
  
  It's just for rustfmt to indent one more depth.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8403/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8403

To: yuja, #hg-reviewers, Alphare
Cc: Alphare, mercurial-devel
phabricator - April 14, 2020, 12:10 p.m.
Alphare added a comment.


  Because you expect to add an async block or something that will make the diff harder to read, I suppose?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8403/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8403

To: yuja, #hg-reviewers, Alphare
Cc: Alphare, mercurial-devel
Yuya Nishihara - April 14, 2020, 2:15 p.m.
>   Because you expect to add an async block or something that will make the diff harder to read, I suppose?

The `match` will be surrounded by a simple `loop {}`, which was previously
a state machine. And yes, reading the subsequent 10 patches would be painful
without indent-level adjustment.
phabricator - April 14, 2020, 2:18 p.m.
yuja added a comment.


  >   Because you expect to add an async block or something that will make the diff harder to read, I suppose?
  
  The `match` will be surrounded by a simple `loop {}`, which was previously
  a state machine. And yes, reading the subsequent 10 patches would be painful
  without indent-level adjustment.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8403/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8403

To: yuja, #hg-reviewers, Alphare
Cc: Alphare, mercurial-devel

Patch

diff --git a/rust/chg/src/runcommand.rs b/rust/chg/src/runcommand.rs
--- a/rust/chg/src/runcommand.rs
+++ b/rust/chg/src/runcommand.rs
@@ -140,30 +140,31 @@ 
     C: Connection,
     H: SystemHandler,
 {
-    match msg {
-        ChannelMessage::Data(b'r', data) => {
-            let code = message::parse_result_code(data)?;
-            Ok(AsyncS::Ready((client, handler, code)))
-        }
-        ChannelMessage::Data(..) => {
-            // just ignores data sent to optional channel
-            let msg_loop = MessageLoop::resume(client);
-            Ok(AsyncS::PollAgain(CommandState::Running(msg_loop, handler)))
-        }
-        ChannelMessage::InputRequest(..) | ChannelMessage::LineRequest(..) => Err(io::Error::new(
-            io::ErrorKind::InvalidData,
-            "unsupported request",
-        )),
-        ChannelMessage::SystemRequest(data) => {
-            let (cmd_type, cmd_spec) = message::parse_command_spec(data)?;
-            match cmd_type {
-                CommandType::Pager => {
-                    let fut = handler.spawn_pager(cmd_spec).into_future();
-                    Ok(AsyncS::PollAgain(CommandState::SpawningPager(client, fut)))
-                }
-                CommandType::System => {
-                    let fut = handler.run_system(cmd_spec).into_future();
-                    Ok(AsyncS::PollAgain(CommandState::WaitingSystem(client, fut)))
+    {
+        match msg {
+            ChannelMessage::Data(b'r', data) => {
+                let code = message::parse_result_code(data)?;
+                Ok(AsyncS::Ready((client, handler, code)))
+            }
+            ChannelMessage::Data(..) => {
+                // just ignores data sent to optional channel
+                let msg_loop = MessageLoop::resume(client);
+                Ok(AsyncS::PollAgain(CommandState::Running(msg_loop, handler)))
+            }
+            ChannelMessage::InputRequest(..) | ChannelMessage::LineRequest(..) => Err(
+                io::Error::new(io::ErrorKind::InvalidData, "unsupported request"),
+            ),
+            ChannelMessage::SystemRequest(data) => {
+                let (cmd_type, cmd_spec) = message::parse_command_spec(data)?;
+                match cmd_type {
+                    CommandType::Pager => {
+                        let fut = handler.spawn_pager(cmd_spec).into_future();
+                        Ok(AsyncS::PollAgain(CommandState::SpawningPager(client, fut)))
+                    }
+                    CommandType::System => {
+                        let fut = handler.run_system(cmd_spec).into_future();
+                        Ok(AsyncS::PollAgain(CommandState::WaitingSystem(client, fut)))
+                    }
                 }
             }
         }