Windows line endings cause a problem #70

Activity

1. klonos commented on Feb 11, 2023

   

   klonos

   on Feb 11, 2023

   Member

   More actions

   @docwilmot I cannot reproduce this on Mac OS 🤔 ...are you running things on Windows?

   I just tried to convert https://git.drupalcode.org/project/menu_trail_by_path/-/tree/7.x-3.x, and I got no weird (). lines:

    /**
   - * Implements hook_menu()
   + * Implements hook_menu().
     */
    function menu_trail_by_path_menu() {

   Can you please let me know of a D7 module that you were able to successfully reproduce this issue with?
2. klonos commented on Feb 11, 2023

   

   klonos

   on Feb 11, 2023

   Last edited by klonos

   Member

   More actions

   ...in the meantime, I have taken a look at the regex used in coder_upgrade_upgrade_hook_alter():

   '@\*\s+[iI]mplement.*?(hook_.*?)(\(\)|)(\.|)$@m'`
   

   I have the following remarks for moment (untested, but pretty sure about these):

   • There's no need to be doing [iI] if we add the i regex flag at the end - it will try to match case-insensitive.
   • (\.|) close to the end seems to be trying to match a dot or nothing, which can be achieved with \.? (or (\.)? if you want to be safe). That's some odd way to try to match 1 or 0 instances of something - the most common thing to be doing is to use a ? for that, unless there's something specific that we're trying to achieve here that I'm not aware of.
   • Similarly to the previous remark, (\(\)|) also tries to match () or nothing, which can be achieved with (\(\))?
   • Not sure what (hook_.*?) is trying to do, but I believe that the intention was to have (hook_)?.*, which means "try to match hook_ but optionally, then followed by any number of characters"
   • The m flag at the end might be (part of) the culprit here. See https://javascript.info/regexp-multiline-mode for what it does:

     The multiline mode is enabled by the flag m. It only affects the behavior of ^ and $. In the multiline mode they match not only at the beginning and the end of the string, but also at start/end of line.

   Anyway, I will need to be able to reproduce this issue first before I can try things and confirm 100% all of the above. I will also need to have a better understanding of the sequence of things coder_upgrade is trying to do, because as you said @docwilmot:

   If you convert the file to Linux line endings first, this doesnt happen.

   That ^^ may be the answer/solution.
3. docwilmot commented on Feb 11, 2023

   

   docwilmot

   on Feb 11, 2023

   MemberAuthor

   More actions

   This was of course all "Greek" to me. 😄

   😄React with 😄3Reacted by Greg Netsas, Martin Price and izmeez